potiuk commented on PR #27912: URL: https://github.com/apache/airflow/pull/27912#issuecomment-1327368060
cc: @alexott @kazanzhy - I have two important doubts about processing of `;` (and sql stripping in general). 1) (Less important if it works) Stripping ";" I followed original implementations I think and I left I think intact original behviour of stripping in both implementationations. Look at the parameterized tests - and the "DBApiHook" behaves now differently than Databricks Hook in this regard - the Databricks hook always splits the `;` https://github.com/apache/airflow/pull/27912/files#diff-db2c1495ba766a1bad71474f36caeb3e8b4228522571e3229c1c64cede967027R99 Where the standard DBApiHook does not: https://github.com/apache/airflow/pull/27912/files#diff-718d368660d07715b383dd4d94344bc41b835189da39bb95fcecaa3a423e6e47R110 2) The behaviour of cursor and connection opening/closing is different in both cases: * DBAPiHook opens connection, cursors - and iterates over multiple queries with the same cursor (runs execute command for each) https://github.com/apache/airflow/pull/27912/files#diff-6e1b2f961cb951d05d66d2d814ef5f6d8f8bf8f43c40fb5d40e27a031fed8dd7R288 * Databricks Hook opens connection, but then creates a cursor for each sql statement separately: https://github.com/apache/airflow/pull/27912/files#diff-f1c71c8b869a687043b306f047f592e3638b3d74d9ec3c972360cf6c9a7956a9R182 3) I think we miss commiting after last statement in Databricks Hook. The DBApi hook does it https://github.com/apache/airflow/pull/27912/files#diff-6e1b2f961cb951d05d66d2d814ef5f6d8f8bf8f43c40fb5d40e27a031fed8dd7R307 but the DAtabricks one does not. When autocommit is off, I think and no explicit commit given , then the transaction will be rolled back automatically https://peps.python.org/pep-0249/#Connection.close > Note that closing a connection without committing the changes first will cause an implicit rollback to be performed. I am no sure what was the reasoning behind those differences - but from what I see those differences now between those two "run" methods. Question is - whether we have good reasons to behave differently for those cases? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
