Error while parsing literals with backslash

[xccui]

Describe the bug Can't properly parse the statement when

1. Backslash is the last character of a literal;
2. The literal is not the last value of a sequence.

To Reproduce Steps to reproduce the behavior:

1. Example: CCJSqlParserUtil.parseStatements("""INSERT INTO "a"."b"("c", "d", "e") VALUES ('c c\', 'dd', 'ee\')""") (Kotlin)
2. Exception:

net.sf.jsqlparser.JSQLParserException: Encountered unexpected token: "dd" <S_IDENTIFIER>
    at line 1, column 53.

Was expecting one of:

    "&"
    ")"
    ","
    "::"
    "<<"
    ">>"
    "COLLATE"
    "["
    "^"
    "|"

3. Note that CCJSqlParserUtil.parseStatements("""INSERT INTO "a"."b"("c", "d", "e") VALUES ('c c', 'dd', 'ee\')""") won't trigger the exception.

Expected behavior

The literals 'c c\' and 'dd'should be properly parsed.

System

• Database: Oracle
• Kotlin Version: 1.4.21-release-IJ2020.2-1
• Java Version: 1.8
• JSqlParser version: 4.0

[manticore-projects]

First of all a proper documentation of the challenge in H2:

VALUES ('c c\', 'dd', 'ee\');

returns

C1	C2	C3
c c\	dd	ee\

But JSQLParser complains:

I will look into that, there is also another similar case.

[manticore-projects]

The problem is simple: JSQLParser treats both characters ' and \ as escape character (which should be a good thing):

-- correctly parsed with JSQLParser
select 'c c\\', 'dd', 'ee\\'
from dual

Unfortunately, this is not the interpretation by Oracle or H2:

'CC\'	'DD'	'EE\'
c c\	dd	ee\

So as stated in another issue, we would need to define an Parser Configuration Feature, which allows only one Escape character at a time (preferably ').

[manticore-projects]

@wumpz: I read through a lot of stuff in order to find a solution. My current suggestion was to

1. define only one Quoting Character inside the Grammar (an maybe that might be the \)
2. move the Configuration of Quoting Characters outside of the Grammar into the Abstract Parser or Parser Utils
3. there we handle/replace the Quoting Character BEFORE parsing and after parsing.

And if you ask me, we should do the same for the squared brackets and any other phony stuff, because at least to me this looks terrifying:

{ if ( !configuration.getAsBoolean(Feature.allowSquareBracketQuotation) && matchedToken.image.charAt(0) == '[' ) {
         matchedToken.image = "[";
         for (int i=0;i<CCJSqlParserConstants.tokenImage.length;i++) {
            if (CCJSqlParserConstants.tokenImage[i].equals("\"[\"")) {
                matchedToken.kind = i;
            }
         }
         input_stream.backup(image.length() - 1);
       }
    }

And I also suspect that it might have performance implications.

Please let me know what you think about that kind of PRE/POST processing. I could easily implement that but only when you did not oppose to the idea.

[xccui]

Thanks for your insights! @manticore-projects

To temporarily fix this problem, I replace all \ with \\ before parsing the SQL and replace them back after the parsing process. Do you think this is a feasible workaround?

[manticore-projects]

Yes, this was exactly what I have suggested to the @wumpz. As kind of Pre-Processing and Post-Processing outside of the Parser itself.

On Mon, 2021-05-24 at 21:06 -0700, Xingcan Cui wrote:

Thanks for your insights! @manticore-projects To temporarily fix this problem, I replace all \ with \ before parsing the SQL and replace them back after the parsing process. Do you think this is a feasible workaround? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[wumpz]

@manticore-projects To be honest I have no idea how you want to externalize this, since that would result in some kind of optional tokens. There was a huge discussion about this at least for square brackets (#677). For this escaping you have to change token definition. So it would a great benefit to do it, but the only way I saw was this backup token thing you mentioned.

[manticore-projects]

What I was refering to was to Modify the SQL String in Java before handing it over to the Parser itself. Something like simple Prep- Parser.  I do this for the Comments already.

So we would transform:

select 'a' 

into

select 'a\'

then parse it.

But we would have to wrap a few Methods, such as Statement(), Statements() and the Constructors() and this is the reason why I asked if you are ok with that approach. And also we would need to change all getName() methods of the Objects and ensure returning the correct name (based on the requested Quoting Character).

On Tue, 2021-05-25 at 15:33 -0700, Tobias wrote:

@manticore-projects To be honest I have no idea how you want to externalize this, since that would result in some kind of optional tokens. There was a huge discussion about this at least for square brackets (#677). For this escaping you have to change token definition. So it would a great benefit to do it, but the only way I saw was this backup token thing you mentioned. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[wumpz]

Ah. Ok. I get it. I was more referring to this bracket thing.

I prefer to build a correct parser. This kind of wrappers feels like a hack.

[manticore-projects]

Ok, clear. So I won't waste my time. Although I do not see a good solution then because onces your token is defined you are stuck with it.

JavaCC21 seems to have implemented Context Sensitive Phony Tokens and I also got our Grammar running on JavaC21. But I still have concerns on the performance and the maturity of JavaCC21. I would like to conclude on the outstanding PRs this week and slize them into smaller pieces making it easier for you to digest.

Maybe you would like to release the 4.1 by June because the performance fixes affect a lot of other applications (like DBeaver, Squirrel SQL).

And maybe then we would like to discuss if JavaCC21 was a strategical option (compiling a Pros/Cons list). 

On Wed, 2021-05-26 at 16:31 -0700, Tobias wrote:

Ah. Ok. I get it. I was more referring to this bracket thing. I prefer to build a correct parser. This kind of wrappers feels like a hack. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[wheredevel]

@wumpz,

I didn't get if there's an issue here or not. Because, I'm able to successfully parse:

SELECT 'c c\', 'dd', 'ee\'

and also this

assertSqlCanBeParsedAndDeparsed(
			"INSERT INTO \"a\".\"b\" (\"c\", \"d\", \"e\") VALUES ('c c\', 'dd', 'ee\')"
		);

If you solved this in the past, could you, please, explain how was it achieved? A reference to the corresponding commit?

[wheredevel]

@wumpz,

I didn't get if there's an issue here or not. Because, I'm able to successfully parse:

SELECT 'c c\', 'dd', 'ee\'

and also this

assertSqlCanBeParsedAndDeparsed(
			"INSERT INTO \"a\".\"b\" (\"c\", \"d\", \"e\") VALUES ('c c\', 'dd', 'ee\')"
		);

If you solved this in the past, could you, please, explain how was it achieved? A reference to the corresponding commit?

Let me elaborate on my own question: TokenManager is supposed to emit a result of the longest matching regex.
So, for example in:

SELECT 'c c\', 'dd', 'ee\'

that would be

'c c\', '

because \' is an escaped ' - so matching shouldn't stop on it.

So, please, explain how did you manage to persuade JavaCC to parse the above corectly?

[manticore-projects]

@wumpz,

I didn't get if there's an issue here or not.

There is the issue, that \ is understood as escape character in JSQLParser (always), but not by every RDBMS. (Similar to Regex and Java Strings, where you will always have to add a \ in the Java String, which won't be present in the original Regex.)

Because, I'm able to successfully parse:

SELECT 'c c\', 'dd', 'ee\'

Which version of JSQLParser are you using? It is possible that you use a Version, which does not define \ as escape character yet.

If you solved this in the past, could you, please, explain how was it achieved? A reference to the corresponding commit?

It is not about solving anything, but about the introduction of a feature (which is escaping as per \). We could simply remove that feature, but then we could not escape properly anymore.

For a really good solution, we would make the escape character configurable (e. g. \ or '). Unfortunately that is not easy and never clean because of the nature of JavaCC and the way, how Token work. Once a Token is defined in the grammar it is final and the Parser will be generated with exactly this Token.

[wumpz]

Could it be as simple as this, that you tried something like this:

assertSqlCanBeParsedAndDeparsed("SELECT 'c c\', 'dd', 'ee\'");

Here we have a single backslash escaping the quotation which results in a quotation. So the parser gets effectively

SELECT 'c c', 'dd', 'ee'

The problematic statement is

assertSqlCanBeParsedAndDeparsed("SELECT 'c c\\', 'dd', 'ee\\'");

where the parser indeed gets a backslashed quotation. And this statement is unfortunately not (yet) parseable. Your understanding of the regular expression is right and it fails like this:

Call:	Statement
  Call:	SingleStatement
    Call:	Select
      Call:	SelectBody
        Call:	SetOperationList
          Call:	PlainSelect
            Consumed token: <<K_SELECT>: "SELECT" at line 1 column 1>
            Call:	getOracleHint
            Return: getOracleHint
            Call:	SelectItemsList
              Call:	SelectItem
                Call:	SelectExpressionItem
                  Call:	Condition
                    Call:	SQLCondition
                      Call:	SimpleExpression
                        Call:	ConcatExpression
                          Call:	BitwiseAndOr
                            Call:	AdditiveExpression
                              Call:	MultiplicativeExpression
                                Call:	BitwiseXor
                                  Call:	PrimaryExpression
                                    Consumed token: <<S_CHAR_LITERAL>: "\'c c\\\', \'" at line 1 column 8>
                                  Return: PrimaryExpression
                                Return: BitwiseXor
                              Return: MultiplicativeExpression
                            Return: AdditiveExpression
                          Return: BitwiseAndOr
                        Return: ConcatExpression
                      Return: SimpleExpression
                    Return: SQLCondition
                  Return: Condition
                  Call:	Alias
                    Call:	RelObjectNameWithoutStart
                      Call:	RelObjectNameWithoutValue
                        Consumed token: <<S_IDENTIFIER>: "dd" at line 1 column 17>
                      Return: RelObjectNameWithoutValue
                    Return: RelObjectNameWithoutStart
                  Return: Alias
                Return: SelectExpressionItem
              Return: SelectItem
            Return: SelectItemsList
          Return: PlainSelect
        Return: SetOperationList
      Return: SelectBody
    Return: Select
  Return: SingleStatement
Return: Statement

The real solution to this case would be to introduce a fallback. The parser should be preconfigured not to use this \' escape. Then parser (like for bracket quotation) should look into the token (<S_CHAR_LITERAL>: "\'c c\\\', \'" at line 1 column 8>) and if this escaping is not allowed should crop this literal (<S_CHAR_LITERAL>: "\'c c\\\'" at line 1 column 8>).

Unfortunately, there is no solution for both cases at the same time. I used (IMHO) the actual way with the most succeeding cases (without counting).

[wheredevel]

@wumpz ,

Yep, I already figured that. Had to print char-by-char in CharStream to discover single \ is not interpreted correctly.

I want to understand: is it correct what @manticore-projects says:

we would need to define an Parser Configuration Feature, which allows only one Escape character at a time (preferably ').

In this example:

SELECT 'c c\', 'dd', 'ee\'

Is the idea to tell in the config \' is no longer an escape for ' ?

Now, I came from the following case that didn't parse:

SELECT * FROM T WHERE t1 LIKE '' ESCAPE '\' AND (t2 = 'a')

I think the above config alone wouldn't help here either. Once \' is allowed as an escape in tokenization - the above will fail. It would "eat" '\' AND (t2 = ' as a token, and fail on the a'. On the other hand, we could contextually tokenize the char following ESCAPE in the grammar differently. I tried to solve it, but it's different from this code, for example:

{ if ( !configuration.getAsBoolean(Feature.allowSquareBracketQuotation) && matchedToken.image.charAt(0) == '[' ) {
         matchedToken.image = "[";
         for (int i=0;i<CCJSqlParserConstants.tokenImage.length;i++) {
            if (CCJSqlParserConstants.tokenImage[i].equals("\"[\"")) {
                matchedToken.kind = i;
            }
         }
         input_stream.backup(image.length() - 1);
       }
    }

because this code is a part of TokenManager (with all the variables and methods in scope). While I'm trying to do similar thing in the parser context:

    	...
    	<K_ESCAPE>
    	{
    	    	//...
    	}

Can you suggest how to solve it?

[manticore-projects]

The idea was to:

1. Define one Quote Character (default maybe ')
2. to allow setting this Quote Character when calling the parser (as configuration) (so either \ or ' was the Quote Character)
3. to parse accordingly

The challenge is, that we would need to modify the Token based on this Configuration but that is not possible. Inside a production we can use LOOKAHEAD for simple IF .. ELSE .. instructions. But there is no such thing for Token definitions, they are constants and the Parser will be built for these constants.

[manticore-projects]

There are 2 alternatives:

a) 2 different Grammar definitions, one for Quote Character ' and one for \, then generate 2 Parsers and select the applicable one as per Configuration

b) use only one predefined Quote Charater in the Grammer like ' and replace the actually desired Quote Character in the SQL before and after Parsing

The same would apply for Statement Separator (if you do not want to use ;).

[wumpz]

I tested a variant of your problem SQL with Postgresql:

SELECT * FROM T WHERE t1 LIKE 'test\m' ESCAPE '\'

Using this I could confirm, that the database looked for something like 'testm'. The escaping is in place But Postgresql does not parse this

SELECT * FROM T WHERE t1 LIKE 'test\'' ESCAPE '\'

Therefore at least for this database using Escape you are not able to switch escaping of quotes. To be honest I somehow expected that, because as you are describing one has to be context sensitive even backwards context sensitive to solve this which would make the parsing much more complex.

Switching the escape sequence in Postgresql is IMHO only possible using Posix styled string literals like

 SELECT * FROM T WHERE t1 LIKE E'test\''

This one at least could be tackled by a token definition.

[wumpz]

So the question is, is this statement of yours with a quotation correct? For which database?

SELECT * FROM T WHERE t1 LIKE 'test\'' ESCAPE '\' AND (t2 = 'a')

[wheredevel]

So the question is, is this statement of yours with a quotation correct? For which database?

SELECT * FROM T WHERE t1 LIKE 'test\'' ESCAPE '\' AND (t2 = 'a')

SQL Server. Like in this ticket, for example.

[wheredevel]

This is my hack:

    public void CommonTokenAction(Token t) {
		String STR = "\'\\\'";
	   	if(t.image.length() > 3 && t.image.startsWith(STR)) {
	   		input_stream.backup(t.image.length() - STR.length());
	   		t.image = STR;
	   	}
		t.absoluteBegin = getCurrentTokenAbsolutePosition();
		t.absoluteEnd = t.absoluteBegin + t.image.length();
	}

mostly works... :)

I think it should be further conditioned on a "parsing state" (which is not something provided by JavaCC as oppsed to lexical state).
For example, "we're now parsing ESCAPE char". I understand how this can be added, but, for now, it's good enough.
I'll be happy to have your comments on that.

[manticore-projects]

Pavel,

thank you for your contribution. Although I am just an interested observer, please allow me to ask:

1. What does "mostly works" mean? What are the constraints?
2. What are the use cases where it fails?

Cheers!

[wheredevel]

For example, these now fail:

net.sf.jsqlparser.statement.select.SelectTest.testIssue167_singleQuoteEscape()
net.sf.jsqlparser.statement.select.SelectTest.testIssue167_singleQuoteEscape2()

To complete the solution (and allow the above to parse correctly) I would limit this solution to "parsing state" - in my case the state should say "we're now parsing ESCAPE char". An example solution is to have such a flag in the parser and turn it on during LikeExpression parsing (after ESCAPE and turn off later).

But, even after that, it solves my case, but not this one:

SELECT 'c c\', 'dd', 'ee\'

[wheredevel]

A little better version - to NOT fail the above mentioned tests. (Still no "parsing state" used).

    public void CommonTokenAction(Token t) {
		String STR = "\'\\\'";
	   	if(t.image.length() > 3 && t.image.startsWith(STR) && Character.isWhitespace(t.image.charAt(3))) {
	   		input_stream.backup(t.image.length() - STR.length());
	   		t.image = STR;
	   	}
		t.absoluteBegin = getCurrentTokenAbsolutePosition();
		t.absoluteEnd = t.absoluteBegin + t.image.length();
	}

[wumpz]

@wheredevel This statement does not run on SqlServer

SELECT * FROM T WHERE t1 LIKE 'test\'' ESCAPE '\' AND (t2 = 'a')

for the same reasons like Postgresql. \' is not valid in SqlServer as well in standard string literals. Using escape you are only replacing the escape character for escapable characters :).

Therefore a complete solution seems to be:

1. to integrate an option to disable an escape single quote and then ala bracket parsing correct the accepted token. Since the used regular expression for our token accepts more or the correct length.
2. introduce a new quoted single character for escape, to insert a backslash without thinking it would escape something here

[wheredevel]

Tried this:

SELECT 
  *
FROM 
  sakila.customer 
WHERE 
  first_name NOT LIKE 'ID\\A' ESCAPE '\' AND last_name = 'ANDREWS'

Works on SQL Server.

[wumpz]

@wheredevel That's right, but you are not escaping a single quote. The problem here is to parse escape '\'. That's why I introduced point 2.

[wheredevel]

@wheredevel That's right, but you are not escaping a single quote. The problem here is to parse escape '\'. That's why I introduced point 2.

This is a legit query, it runs and produces results:

SELECT 
  *
FROM 
  sakila.customer 
WHERE 
  first_name LIKE 'IDA' ESCAPE '\' AND last_name = 'ANDREWS'

So, ESCAPE '\' should be parsed here...

[wumpz]

@wheredevel

The problem here is to parse escape ''. That's why I introduced point 2.

[revusky]

mostly works... :)

I don't know whether you're aware of this (maybe you are aware of it, but your comment shows no awareness of it...) but the CommonTokenAction disposition in legacy JavaCC is quite fundamentally broken. I explain this in gory detail here. The main reason is that it doesn't work in conjunction with LOOKAHEAD.

You see, when you're in a lookahead routine,, the tokens are cached in the token.next and token.next.next etc. fields so, when you're parsing whatever construct, it is quite possible that there was a previous lookahead routine that already tokenized the input a certain way. In such a case, these tokens are cached and your CommonTokenAction will simply never be called. The aforementioned article explains that and how this is resolved in JavaCC21.

That is the main problem with the legacy CommonTokenAction but another problem is that you can only have ONE of them! So if you're munging tokens a lot, you end up with a very big CommonTokenAction and the whole thing tends to be become quite unmaintainable. In JavaCC21, you inject a TOKEN_HOOK method and it generates a unique name for it. So you can have as many of them as you want. Actually, that becomes a very big consideration in JavaCC21, because it has an INCLUDE directive. So if you include a grammarfile that already has a CommonTokenAction method defined, then there is a real first-order problem! Of course, that doesn't happen in legacy JavaCC for the simple reason that it does not have an INCLUDE directive!

In other matters, Pavel, after noticing that you starred the JavaCC21 project on github, and I wrote you an email at the address bp*****e(at)gmail.com. That was on April 18. Did you receive that email?

[revusky]

Hi Andreas. For a variety of reasons, I am a bit reluctant to weigh in on these things, because finally, what I can I say? People just have to figure out things for themselves, I guess... but in this case, finally, I feel like I have to make certain points about this.

But I still have concerns on the performance and the maturity of JavaCC21.

Andreas, based on my interaction with you over the last month, you're not a bad guy, but I have no choice but to point out that the above sentence embodies some outright logical fallacies. For starters, consider the old English homily about not looking a gift horse in the mouth. You know, in German: Einem geschenkten Gaul schaut man nicht ins Maul! Now, it's not my main point here, but I would say in passing that, even just on that level, if I've done all this work of fixing every outstanding issue that there is in the old crufty legacy JavaCC... and I have... then... it's a gift! Take it, use it, be happy... already, there is just that common-sensical level of things in the situation.

BUT again, that's not my main point even.... no, by all means... do look the gift horse in the mouth... sure, be my guest... But I would put it to you: Irrespective of what you find when you look the gift horse in the mouth, can that ever justify a decision to hitch your cart to a horse that is DEAD!!??

You see, the implicit fallacy is just too glaring for me to let it slide with no comment. And there are other problems here. One really does have to understand that legacy JavaCC and JavaCC 21 are not two different projects in the same way that ANTLR and JavaCC are two different projects. There is no technical reason for JavaCC and JavaCC 21 to be two separate projects. This state of affairs came about for entirely non-technical reasons -- specifically the intransigence and overall uselessness of the people sitting on the existing JavaCC project. I'm loath to repeat myself too much which is why I wrote the whole sordid history of the thing here.

So, the bottom line is that JavaCC 21 is simply a more advanced version of the legacy JavaCC project. So, to be talking about its "maturity" is really a bit much. However "mature" or "immature" it is, it is necessarily more mature than a more backward version of the SAME THING! And again, whatever problems you see when you look the gift horse in the mouth, that never is an argument for opting for a horse that is DEAD!

But it also seems like it's based on a conceptual confusion between something being "mature" and just being "old". A group of people, all born in 1996, are all of the same chronological age, but "maturity" is obviously another question. That varies. Drastically. The legacy JavaCC project was created in 1996 and developed actively for just a couple of years after that. The JJTree part is probably from the first half of 1997. And there just has not been a meaningful new feature since then! And I think it is pretty safe to say that no real development has taken place since 1998. Well, maybe 1999 at the latest... Regardless, it was basically abandoned in whatever half-baked state it was in until mid-2003 when Sun decided to open-source it. Unfortunately, the utterly wrong kinds of people took hold of the thing and nothing was ever really done. (No need to take my word for that. Just scan through their commit history.) JavaCC is kind of an archetypal nothingburger project.

In short, this is a project that yes, is old, but it is not mature at all! There is a period of a couple of years, maybe 1996-1998 in which it was actively developed and the real truth of the matter is that it never had a legitimate 1.0 release. For them to say that they are entering an 8.x release cycle is IMHO very hard to characterize as anything other than an outright fraud! If it was honestly labeled, the version number, when it was open-sourced in 2003, would have been about 0.3, and in the following 18 years, there has not been enough forward movement to justify taking it to 0.4. Granted version numbers are inherently arbitrary, but I think what I say basically stands...

So, to be expressing doubts about the maturity of a tool that is simply a more advanced version of that thing, that was NEVER mature in any real sense in the first place.... It's really a complete misrepresentation of what is really going on here.

Now, to be completely fair, it is correct to say that some parts of the current JavaCC21 codebase are immature. For example, the new machinery for generating fault-tolerant parsers could be described quite accurately as not very mature. BUT... in the context of a comparison (or implicit comparison) with the legacy JavaCC, this is totally beside the point because this is functionality the legacy tool simply does not have!

And it never will. (You could bet your house that.)

Okay, so there it is. I feel I've said what needs to be said. But, in closing... cutting to the chase, as it were.... just consider the following question.

What is the use case for the legacy JavaCC?

And I mean technically speaking, of course. Why would somebody ever opt to use the legacy JavaCC in the full knowledge that JavaCC21 exists? Just for example, why would you use a version of JavaCC that only supports the Java language up to JDK 7 (10 years old at this point) in the full knowledge that there is a version available that is up to date with JDK 16. Or, as I just explained to Pavel Beckerman, how CommonTokenAction basically doesn't work in legacy JavaCC. (Well, sometimes, maybe... not really....) Why would you ever use the legacy JavaCC in the full knowledge that this is broken AND _in the full knowledge that there is a fixed version?

By the way, it should be needless to say, but when I pose the above question, I do mean based on real, legitimate technical considerations... For example, that there is some set of people who want to pretend that they're doing something and, for some mysterious reason, you don't want to hurt their feelings.... that would not count, in my view as a "legitimate technical consideration". You know what I mean... By the same token, something like "I don't personally know the guy but somebody told me that that Revusky guy is a real asshole"... that would not be a "legitimate technical consideration" either.

So I leave that question open to anybody who happens on this. It's not just for Andreas here. I have to think that just trying to focus on that question, what is the real use-case for the legacy JavaCC, that might really just shed some light on the situation, no?