Closed seanlinsley closed 1 year ago
Then what do we do, stop storing the queries and warn users that some queries were dropped?
If our solution is to not store these queries, we should revert parts of https://github.com/pganalyze/libpg_query/pull/155 in order to resolve https://github.com/pganalyze/pg_query/issues/266. That parsing change was made in part to "fix" (ignore) the normalization issue on our end.
Update: continuing the discussion internally
I still think there's value in properly normalizing this type of SQL query, but I chose this unwrapping approach because the parse tree doesn't contain all of the information needed to normalize the function body in place (IIRC).
Closing for now as we've found an internal workaround to this issue.
For the record, I was wrong about insertQuery
being uncalled in Sequelize: I still could not trace how the code gets called, but apparently it does get called in findOrCreate
. See an issue related to that here: https://github.com/sequelize/sequelize/issues/14266 .
Immediately invoked functions are created, called, and then dropped all in the same query. https://github.com/pganalyze/libpg_query/pull/155 changed query normalization to drop the function contents, generating an unparseable query that is missing all relevant parts for a reader.
Ideally libpg_query would parse function bodies automatically so normalization would work as intended (https://github.com/pganalyze/libpg_query/issues/148), but in lieu of that, this PR pulls out the function body so it can be normalized on its own.
If we want to preserve the wrapping function, it should be possible to insert a sentinel value into the parse tree to later be replaced with the properly normalized version of the function body that this generates.
Todo:
&ParseResult
instead of&str
to avoid parsing when that has already been doneunwrap()
call took()?
, or maybe returnResult<Option<String>>