Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17553 )
Change subject: IMPALA-10557: Support Kudu's multi-row transaction ...................................................................... Patch Set 19: (3 comments) http://gerrit.cloudera.org:8080/#/c/17553/19/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/17553/19/fe/src/main/java/org/apache/impala/service/Frontend.java@1722 PS19, Line 1722: don't nit: "doesn't" or "does not", same below. http://gerrit.cloudera.org:8080/#/c/17553/19/fe/src/main/java/org/apache/impala/service/Frontend.java@1818 PS19, Line 1818: if (queryOptions.isEnable_kudu_transaction()) { : // Kudu don't support transaction for DELETE/UPDATE statements now. : if (analysisResult.isUpdateStmt() : && analysisResult.getUpdateStmt().isTargetTableKuduTable()) { : throw new TransactionException( : "Kudu don't support transaction for UPDATE statement."); : } else if (analysisResult.isDeleteStmt() : && analysisResult.getDeleteStmt().isTargetTableKuduTable()) { : throw new TransactionException( : "Kudu don't support transaction for DELETE statement."); : } : } Just curious -- today Kudu will return an error if trying to update or delete in the context of a transaction. Would it make sense to rely on that, and simply pass all operations through to Kudu? If any of them fail, they would be caught as row errors by Impala and fail the transaction anyway. That way, we wouldn't have to update this code much as transactions support expands. http://gerrit.cloudera.org:8080/#/c/17553/14/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/17553/14/tests/custom_cluster/test_kudu.py@388 PS14, Line 388: self.execute_query(self._delete_query.format(table_name)) : self.execute_query(self._insert_3_rows_query.format(table_name)) : cursor.execute(self._row_num_query.format(table_name)) : assert cursor.fetchall() == [(3,)] : self.execute_query(self._insert_select_query.format(table_name)) : cursor.execute(self._row_num_query.format(table_name)) : assert cursor.fetchall() == [(103,)] : : @pytest.mark.execute_serially : @SkipIfKudu.no_hybrid_clock : def test_kudu_txn_not_implemented(self, cursor, unique_data > Changed the frontend code to return an error if UPDATE/UPSERT/DELETE is ran Thank you for updating this! -- To view, visit http://gerrit.cloudera.org:8080/17553 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I876ada48991afdff5d61b5d6a0417571aba7cb34 Gerrit-Change-Number: 17553 Gerrit-PatchSet: 19 Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Wed, 16 Jun 2021 18:43:42 +0000 Gerrit-HasComments: Yes
