Bug 420599

Thanks Jordi for the greatly explained issue, as always.

This time I am unsure it applies here. See #376052 and particular solution at 
 https://bugs.kde.org/show_bug.cgi?id=376052#c2.

We have this autotested already as well.

We're victims of insufficent docs for the KDbSQL dialect.

PS: There are two consumers of such docs:
- KEXI users
- KDb users.

It would be good to have a single "KEXI SQL documentation" chapter (kind of appendix) and appear, accordingly, both in KDb Doxygen docs and KEXI's handbook.
I am unsure what format would be good for that, maybe markdown. Things are also undecided recently also technical-wise because user docs based on wikis are not the most modern and convenient tech today (https://userbase.kde.org/Kexi/Handbook).

For the example query given in https://bugs.kde.org/show_bug.cgi?id=420599#attach_127876

This works: SELECT group FROM source
(because KDb escapes indentifiers in the driver's dialect and there is no confusion for the parser)

This should work 

SELECT [group] FROM source

once this wish is implemented: https://bugs.kde.org/show_bug.cgi?id=332161

Don't forget to read conclusion from there. And possible find disadvantages.

(As of KDb 3.2.0 [group] means unconditionally a query parameter)

PS: Any such improvements are apparently KDb-only. KEXI would follow.

BTW the task for the bug #332161 is open for adoption.

Sorry if i misunderstood it, but from your comment that says

> This time I am unsure it applies here. See #376052 and particular solution at 
> https://bugs.kde.org/show_bug.cgi?id=376052#c2.

it seems like you are talking about the conversion from KDbSQL dialect to the driver’s dialect.
That works as expected because, as you noted, the query

SELECT group FROM source

does indeed produce the expected result.
However, what i am reporting is a problem with the SQL that Kexi stores in its kexi__objectdata table for queries.

In fact, i did not write the SQL query in the attached file; i only used the designer mode and switched to text view just to debug, as it were, the results.

My understanding is that KexiQueryDesignerGuiEditor uses a KDbQueryScheme to build the SELECT query without resorting to string manipulations, but when it is time to save the query and serialize it into kexi__objectdata uses a KDbNativeStatementBuilder to create the SQL string. KDbNativeStatementBuilder calls KDb::escapeIdentifier to escape, if needed, field and table names and, in this case, recognizes the group column as a SQL keyword and escapes it as "group" — KDb::escapeIdentifier always uses double quote. Then, to load the query in data mode, Kexi reads the serialized SQL from kexi__objectdata with KDbParser that recognizes "group" as the string literal 'group' instead of a column name.  Afterwards, this query is converted to the driver-specific dialect, but at that moment it is too late and the column is now an anonymous literal.

One source of confusion is that i dot know if the “native” in KDbNativeStatementBuilder refers to driver-native or KDbSql-native statement. If the former, the problem is from Kexi and not KDb, but then KDbNativeStatementBuilder should use KDbConnection::escapeIdentifier instead of KDb::escapeIdentifier. If the latter, KDbNativeStatementBuilder should build its statements in a way that KDbParser should be able read correctly.


> This should work 
>
> SELECT [group] FROM source
>
> once this wish is implemented: https://bugs.kde.org/show_bug.cgi?id=332161

Using [] to escape identifiers would free the double quotes to be used only for string literals, but it seems to me that KDbNativeStatementBuilder, or KDB::escapeIdentifier, should still be corrected to use the proper KDbSql escape characters.


Again, sorry if i misunderstood your response.

The SQL string in kexi__objectdata is in KDBSQL dialect. This makes the KEXI format backend independent and the database content can be migrated without any change e.g. from a file to a server. I am sorry - it must be too deeply hidden behind the mask.

So that string, when retrieved from kexi__objectdata, is displayed as-is in the SQL view of KEXI because it's assumed to be the portable KDBSQL dialect. (Well, in recent KEXIs (>=3.0 probably) it's actually parsed to check validity and if we're switched from the visual editor, it's formatted as well).

> One source of confusion is that i dot know if the “native” in KDbNativeStatementBuilder refers 
> to driver-native or KDbSql-native statement.


In KexiQueryDesignerGuiEditor::storeLayout() there's 

    KDbNativeStatementBuilder builder(conn, KDb::KDbEscaping);

This means that we store the KDbSql-native statements. The generateSelectStatement() has null driver in the logic everywhere in this case.
I admit the KDbNativeStatementBuilder class name can be confusing in a way that it serves both native SQLs and the KDBSQL. Original purpose was for the native only but then it was soon apparent that large portions of code are common for KDBSQL and native SQLs.

KDbNativeStatementBuilder started as an internal class but now in >=3.1 it's a public API. Therefore renaming to a better name, maybe KDbStatementBuilder, is not possible :)

> Using [] to escape identifiers would free the double quotes to be used only for string literals, but 
> it seems to me that KDbNativeStatementBuilder, or KDB::escapeIdentifier, should still be corrected to 
> use the proper KDbSql escape characters.

Is this still the case given what I said above?

>> Using [] to escape identifiers would free the double quotes to be used
>> only for string literals, but it seems to me that
>> KDbNativeStatementBuilder, or KDB::escapeIdentifier, should still be
>> corrected to use the proper KDbSql escape characters.
> 
> Is this still the case given what I said above?

I believe it is, yes.

When i open the query from the attached database, using Kexi 3.2 or 3.3, in text view it is true that the displayed SQL remains unchanged, as you said, but when switching to data view all the rows have a value of 'group', even though the source table, the one with the "group" column, has only integers; design view the expression under “query columns” is now “expr1: 'group'”.

Moreover, if i make any change to the query in design mode, for example toggling twice the visible check box, and *then* switch to text view without saving the query, it changes again to SELECT 'group' AS expr1 FROM source.

Checking with sqlite3 the value stored in kexi__objectdata, it outputs:

> sqlite> select * from kexi__objectdata where o_sub_id = 'sql';
> 2|SELECT "group" FROM source|sql
> sqlite> 

If i am not mistaken, it means that KDbNativeStatementBuilder generated a statement with the column quoted as "group" straight from the KDbQuerySchema, but KDbParser reads it as if it were:

SELECT 'group' AS expr1 FROM source

Thus, even if KDbParser could read columns quoted [as this], unless KDbNativeStatementBuilder used [ and ] as quote characters, the problem would still be there, wouldn’t it?

Sorry if a am not making any sense; i must not understand something from KDb internals :)

> When i open the query from the attached database, using Kexi 3.2 or 3.3, 
> in text view it is true that the displayed SQL remains unchanged, as you 
> said, but when switching to data view all the rows have a value of 'group', 
> even though the source table, the one with the "group" column, has only 
> integers; design view the expression under “query columns” is now “expr1: 
> 'group'”.

KEXI Query plugin tries to avoid modifications if it's possible or easy.
WHen SQL view is open directly, no SQL is altered.
If the design view is used, sometimes alias is added (this is a feature of KDb) in order to have clearly named all columns. There's such case when we use constants or in general, expressions, instead of column names in the "FROM" SQL section.

This is all still KDBSQL dialect we're within. Native dialects are touched only when data view is involved, so - as expected - in a lazy manner.

> Moreover, if i make any change to the query in design mode, for example 
> toggling twice the visible check box, and *then* switch to text view 
> without saving the query, it changes again to SELECT 'group' AS expr1 FROM 
> source.

This is a quality of round-tripping between these two views and can be always improved. I remember a lot of code and states being added to achieve as small changes as possible. 

> If i am not mistaken, it means that KDbNativeStatementBuilder generated a statement with the column quoted as "group" straight from the KDbQuerySchema, but KDbParser reads it as if it were:


There's string  (\'{e_str1}*\'|\"{e_str2}*\") in the parser; type of quotes are unimportant. Double quotes are never meant to be column names (unlike in some SQL dialects) and for 2 more reason this would stay as is: 1. compatibility with prev. KEXIs and MS Access, 2. "group" really looks like a string literal.


> Thus, even if KDbParser could read columns quoted [as this], unless KDbNativeStatementBuilder used [ and ] as quote characters, the problem would still be there, wouldn’t it?

When the #332161 is implemented, KDbNativeStatementBuilder shall support [foo] the way it's designed in #332161, that is, first attempt is to locate identifier foo and if it fails, foo is meant to be a query parameter.