-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pg_query integration #691
Pg_query integration #691
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, read all changes, actually 3 files. see, that here we have new pg_query coder/decoder that handles a_const
tag me and specify files that should be reviewed, please, due to a lot of files have only import path changes
# Conflicts: # cmd/acra-server/common/config.go # cmd/redis.go # encryptor/base/config_loader/config_loader.go # encryptor/base/config_loader/consul/storage.go # encryptor/base/config_loader/filesystem/storage.go
Fixed failing tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i didn't finished review and stopped on 106/120 files, but I feeling that started reading diagonally *( will continue tomorrow
func (filter *SearchableQueryFilter) ChangeSearchableOperator(expr *pg_query.A_Expr) { | ||
switch expr.Name[0].GetString_().GetSval() { | ||
// sqlparser.NullSafeEqualStr, sqlparser.LikeStr, sqlparser.ILikeStr | ||
case "=": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we handle here only =
operator? and stopped handling !=
/<>
? and ignore like/ilike?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice you found it. It can affect the EE part.
|
||
if expr.Rexpr.GetAConst() != nil || expr.Rexpr.GetParamRef() != nil || expr.Rexpr.GetTypeCast() != nil { | ||
if len(expr.Name) == 1 { | ||
if val := expr.Name[0].GetString_(); val != nil && (val.GetSval() == "=" || val.GetSval() == "<>") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we handle !=
? as I remember, postgres accepts it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it accepts it but the parser represents it as <>
. !=
is an alias of <>
https://www.postgresql.org/docs/current/functions-comparison.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember that for pgsql it's the same but not sure should we handle it explicitly here or parser converts !=
-> <>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the parser converts it to <>
# Conflicts: # encryptor/dbDataCoder.go
Integration of pg_query PostgreSQL parser into QueryDataEncryptor component. Split
encryptor
into PostgreSQL and MySQL componentsChecklist
with new changes