Support PostgreSQL double qouted columns #590
Conversation
Added support of double quoted column names for PostgreSQL
Added additional comments
Fixed license formatting
Fixed comments formatting
Added scenario comparison where test = test for MySQL
Added fix related to column names with double quotes in SELECT statements
Restructure sqlparser sql.y file
| input: `select * from mytable where "test" = 1 and 'value' = 'value'`, | ||
| output: `select * from mytable where 'test' = 1 and 'value' = 'value'`, | ||
| }, { | ||
| input: `SELECT "id", "landline_number" AS "landlineNumber", "removal" FROM "users" AS "User" where "User"."is_active"`, |
There was a problem hiding this comment.
lets add some somparison too: WHERE "User"."AGE" = 123 and simple "AGE"='123' without table name.
There was a problem hiding this comment.
plus I remember that you mentioned some integration tests with double quotes. Can we extend them with additional cases that we caught?
Added more unit tests
| @@ -215,6 +215,11 @@ var ( | |||
| input: `select * from mytable where "AGE" = 1 and "TEST" = 'test'`, | |||
| // postgres allow to use double quote string for columns | |||
| dialect: postgresql.NewPostgreSQLDialect(), | |||
| }, { | |||
| input: `insert into some_table(id, data) VALUES (10918, "test")`, | |||
There was a problem hiding this comment.
lets add comment, that it is temporary valid for MySQL default settings, but invalid for PostgreSQL & MySQL with ANSI mode, and probably will be changed. It will help in the future to understand why this case will fail after future changes)
There was a problem hiding this comment.
Sure, also added TODO in invalidSQL tests that such query should be tests on failure
Added comment about future updates
Fixed matching with Tables for searchable encryption
|
@Lagovas I added a couple of unit tests to cover correct matching with tables from config. Please let me know if you want more tests to be added. |
| "github.com/cossacklabs/acra/sqlparser" | ||
| ) | ||
|
|
||
| func TestGetTableSchemaOfColumnMatchConfiTable(t *testing.T) { |
There was a problem hiding this comment.
| func TestGetTableSchemaOfColumnMatchConfiTable(t *testing.T) { | |
| func TestGetTableSchemaOfColumnMatchConfigTable(t *testing.T) { |
| ) | ||
|
|
||
| func TestGetTableSchemaOfColumnMatchConfiTable(t *testing.T) { | ||
| tableName := "some_table" |
There was a problem hiding this comment.
can we use name with upper-cased characters here and below? to be sure that we handle it correctly. Because previously we handled it in lowercase by default
sqlparser/ast_test.go
Outdated
| quote: 34, | ||
| v: "table", | ||
| } | ||
| b, err := json.Marshal(str) |
There was a problem hiding this comment.
why do we use json package instead of sqlparser.String?
There was a problem hiding this comment.
Just copied from another test.
Fixed after review/add integration tests to cover double quoted tables in search queries
tests/test.py
Outdated
| self.insertRow(context) | ||
| self.insertDifferentRows(context, count=5) | ||
|
|
||
| query = 'SELECT * FROM "test_searchable_transparent_encryption" WHERE searchable = :searchable' |
There was a problem hiding this comment.
maybe lets use "searchable" = ...? to test wrapped column too?
Added double quoted column in the integration test query
This PR contains only
sqlparserchanges to fix the open issue. During debugging we faced that sqlparser flow has incorrect flow for parsing statements asWHERE "column" = 'value'as it looks onvaluerule instead ofcolumn_name.In this PR two new types were introduced
column_name_extwhich is almost ascolumn_namebut has primary DOUBLE_QUOTE_STRING rule to track double quoted column only for PostgreSQL.column_name_expression- which is the same asvalue_expressionbut usedcolumn_name_extinstead ofcolumn_name.Checklist
with new changes