Quote and parse identifiers with square brackets
Needs ReviewPublic
Actions

Authored by jfita on May 1 2020, 7:20 PM.

Details

Reviewers

staniek
piggz

Summary

It has been decided that KDb will use the same characters to quote
identifiers as it uses to delimit query parameters: the square
brackets ([]). Depending on the context will decide if a quoted
identifier is a column name or a query parameter.

That is, given a table named “sample” that has two columns “id” and
“select”, KDb is able to parse the following query

SELECT [table].id AS [integer], [select], [a parameter] FROM sample [table] WHERE [select] = [another parameter] ORDER BY [integer] DESC;

and can tell apart that [a parameter] and [another parameter] are query
parameters while the rest are references to columns or column aliases.

Because query parameters and quoted identifiers share the same
characters, the lexed did not have to change much, but the parser now
needs non-terminal identifier rule to recognize both unquoted and quoted
identifiers. That is why i also had to change the tokens from the lexer.

Most of the time the context is enough to know whether the [identifier]
is a quoted identifier or a query parameter (e.g., in ORDER it can only
be the former), but this can not be done for expressions and there is
the need of a post-parse step to try to resolve the ambiguity.

I first tried to create a KDbQuotedIdentifierExpression that would
behave like a KDbVariableExpression or a KDbQueryParameterExpression
depending on whether KDbExpression::verify would be able to find the
name as a table column or alias.

However i had a lot of trouble making it work without duplicating a lot
of code from both KDbVariableExpression and KDbQueryParameterExpression
and, at the end, i changed the strategy: at first, all quoted
identifiers are assumed to be query parameters and a new
KDbExpression::resolveQuotedIdentifiers would *replace* all query
parameters whose value that can be resolved to a column with a new
KDbVariableExpression.

As far as i could see, Kexi picks up the changes transparently and
stores the SQL in KDBSQL quoted with the new characters, and makes this
change compatible with previous versions.

The only drawback is that, from now own, if a query had a quoted
identifier for a column that the user deletes now that identifiers would
be parsed as a query parameter.

Finally, i also changed the quote character in KDb::escapeIdentifier so
that KDbNativeQueryBuilder would escape all identifiers using brackets
instead of double quotes, otherwise they would be parsed as string
literals instead.

BUG:332161
BUG:420599

FIXED-IN:3.2.1

Diff Detail

Repository

R15 KDb

Branch

332161-autotests

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 26204
Build 26222: arc lint + arc unit

jfita created this revision.May 1 2020, 7:20 PM

Restricted Application added a project: KDb. · View Herald TranscriptMay 1 2020, 7:20 PM

Restricted Application added a subscriber: Kexi-Devel-list. · View Herald Transcript

jfita requested review of this revision.May 1 2020, 7:20 PM

Harbormaster completed remote builds in B26204: Diff 81700.May 1 2020, 7:20 PM

Beware that the SQL query in the summary will not work without the change in D29277 and it will fail with

Error opening database cursor.
Message from server: near "table": syntax error
Server result code: 1 (SQLITE_ERROR)

The following modified query avoids the need for escaping in the SQLite driver and works with the change in this diff:

SELECT id AS [integer], [another parameter] FROM sample WHERE id > [parameter] ORDER BY [integer] desc;

Is not as interesting as the other, though :).

Also note that the changes would have been far less extensive if the characters for query parameters and quoted identifiers were not the same.
I guess you already considered that, but i felt like i should mention it anyway.

Revision Contents
Changeset List

			Path	Packages
M			autotests/ExpressionsTest.h (2 lines)
M			autotests/ExpressionsTest.cpp (74 lines)
M			autotests/OrderByColumnTest.cpp (4 lines)
M			autotests/parser/SqlParserTest.cpp (5 lines)
M			autotests/parser/data/statements.txt (43 lines)
M			autotests/tools/IdentifierTest.cpp (16 lines)
M			src/KDb.cpp (16 lines)
M			src/expression/KDbExpression.h (6 lines)
M			src/expression/KDbExpression.cpp (30 lines)
M			src/expression/KDbQueryParameterExpression.cpp (2 lines)
M			src/parser/KDbParser_p.cpp (6 lines)
M			src/parser/KDbSqlParser.y (62 lines)
M			src/parser/KDbSqlScanner.l (14 lines)
M			src/parser/generated/KDbToken.h (6 lines)
M			src/parser/generated/KDbToken.cpp (4 lines)
M			src/parser/generated/sqlparser.h (6 lines)
M			src/parser/generated/sqlparser.cpp (839 lines)
M			src/parser/generated/sqlscanner.cpp (6 lines)

Commit	Tree	Parents	Author	Summary	Date
b294ab926db9	c74835a30bd5	a85f2c20c8af	jordi fita i mas	Quote and parse identifiers with square brackets (Show More…)	May 1 2020, 12:06 PM

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	81700	a85f2c2		May 1 2020, 7:20 PM	★	★

Quote and parse identifiers with square bracketsNeeds ReviewPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 81700

autotests/ExpressionsTest.h

autotests/ExpressionsTest.cpp

autotests/OrderByColumnTest.cpp

autotests/parser/SqlParserTest.cpp

autotests/parser/data/statements.txt

autotests/tools/IdentifierTest.cpp

src/KDb.cpp

src/expression/KDbExpression.h

src/expression/KDbExpression.cpp

src/expression/KDbQueryParameterExpression.cpp

src/parser/KDbParser_p.cpp

src/parser/KDbSqlParser.y

src/parser/KDbSqlScanner.l

src/parser/generated/KDbToken.h

src/parser/generated/KDbToken.cpp

src/parser/generated/sqlparser.h

src/parser/generated/sqlparser.cpp

src/parser/generated/sqlscanner.cpp

Quote and parse identifiers with square brackets
Needs ReviewPublic
Actions

Revision Contents
Changeset List