search

babelfish-for-postgresql / babelfish_compass

Babelfish Compass: compatibility assessment tool for Babelfish for PostgreSQL
Apache License 2.0
108 stars 21 forks source link

Parametrized Statements reported as Dynamic SQL #90

Closed bfcamara closed 1 year ago

bfcamara commented 1 year ago

I am trying to run compass against an extended events file generated while many .NET apps are running connecting to a big MS SQL Server instance. Client apps are using parameterized statements, which is considered a good practice (preventing sql injection, execution plan reuse, etc.). A parametrized query is mapped to a rpc event using the sp_executesql.

For example:

exec sp_executesql N'select id from users where Email=@email', N'@email varchar(200)', @email = 'test@mydomain.com'

These statements are being reported as dynamic sql, which need to be reviewed manually. This makes really hard to assess about the feasibility of the migration to Amazon Babelfish Aurora.

To workaround this issue, I needed to manipulate the xml file to replace all sp_executesql by the original statement. I'm wondering if this is the expected behavior of the tool. I think the majority of the statements in apps are parameterized, hence I guess it might make sense to have the parser recognizing the content of the sp_executesql.

Any thoughts?

robverschoor commented 1 year ago

Extracting the SQL statement from sp_executesql and analyzing that as a statement rather than treating it as a string, is an outstanding development item. For cases where the majority of the captured SQL consists of sp_executesql, the usability is currently indeed limited. Apparently your case falls into this category.

bfcamara commented 1 year ago

Extracting the SQL statement from sp_executesql and analyzing that as a statement rather than treating it as a string, is an outstanding development item. For cases where the majority of the captured SQL consists of sp_executesql, the usability is currently indeed limited. Apparently your case falls into this category.

Got it. I would say that those cases is the majority. In the .NET world it's the typical use case, using parameterized queries with ADO.NET, Dapper, Entity Framework, etc.

For now my workaround is to apply a couple of regexp to extract the content of sp_executesql and replace its usage directly in the events file.

robverschoor commented 1 year ago

The latest version of Compass (v.2022-12) has just been released, and this version will analyze a dynamic SQL query just like the other SQL code if the query is a string literal. The executing statement is not flagged for further review. If a variable is involved, there is no change and the executing statement will be marked as 'must review manually'. The above is done for EXECUTE(), sp_executesql, sp_prepare, sp_prepexec, sp_cursorprepare, and sp_cursorprepexec.

Note that this latest version of Compass also supports the upcoming Babelfish release v.2.3.0, though that release is not available yet (but stay tuned).