Normalize and unwrap immediately invoked functions

[seanlinsley]

Immediately invoked functions are created, called, and then dropped all in the same query. pganalyze/libpg_query#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 (pganalyze/libpg_query#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:

• accept a &ParseResult instead of &str to avoid parsing when that has already been done
• change unwrap() call to ok()?, or maybe return Result<Option<String>>

[msakrejda]

I looked into where this is coming from on the Sequelize side. It looks like the source for this is this code in the insertQuery method (note the "// Mostly for internal use, so we expect the user to know what he's doing!" comment on the option that triggers this behavior). Further, the method is marked @private in the jsdoc comment, and indeed, it's missing from the relevant documentation. I tried to see if it was ever a public API, but history was lost in a recent monorepo-ification, and a changelog search doesn't turn up anything either.

Reviewing the Sequelize code, I don't believe there's another way to hit this code path (when working with Postgres).

I think given that this is not part of a documented public API in Sequelize, that other client libraries don't seem to be doing anything similar, and that this is such idiosyncratic behavior (this seems like it would be better handled with savepoints or ON CONFLICT in Postgres), unless we see more of this, I would suggest we avoid going out of our way to support it.

[seanlinsley]

Then what do we do, stop storing the queries and warn users that some queries were dropped?

[seanlinsley]

If our solution is to not store these queries, we should revert parts of pganalyze/libpg_query#155 in order to resolve pganalyze/pg_query#266. That parsing change was made in part to "fix" (ignore) the normalization issue on our end.

Update: continuing the discussion internally

[seanlinsley]

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.

[msakrejda]

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: sequelize/sequelize#14266 .