add new field updatedRows to QueryStatistics.java where it's availabl… #24715
Conversation
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
1 similar comment
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Hi, Could you please take a look? |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
bhzaeri
force-pushed
the
event-listener-updated-rows-number-2
branch
from
04340f6 to
22385d9
Compare
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
| @@ -121,6 +121,8 @@ public Page getOutput() | |||
|
|
|||
| OptionalLong rowsUpdatedCount = operation.execute(); | |||
|
|
|||
| operatorContext.recordUpdatedPositions(rowsUpdatedCount.orElse(0L)); | |||
There was a problem hiding this comment.
I think you may also need to take care of the rowCount in the MergeWriterOperator
There was a problem hiding this comment.
How can I run into the MergeWriterOperator? Should I run a MERGE query?
bhzaeri
force-pushed
the
event-listener-updated-rows-number-2
branch
from
22385d9 to
1ee7ced
Compare
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
@cla-bot check |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
The cla-bot has been summoned, and re-checked this pull request! |
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued.
bhzaeri
force-pushed
the
event-listener-updated-rows-number-2
branch
from
1ee7ced to
0f6ec97
Compare
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Before this change, only one-way match was verified: that each output symbol is backed by an assignment.
Without trailing path separator the recursive delete operation fails for directory buckets (e.g. S3 Express)
It makes it easier to reason about which session property maps to which configuration property.
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued.
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
github-actions
bot
added
docs
iceberg
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued.
….com/bhzaeri/trino-updated-rows into event-listener-updated-rows-number-2
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued.
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued.
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued.
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued.
Description
The issue that this pull request tries to fix is that after running UPDATE/DELETE queries, the
outputRowsinQueryStatisticsis always 1. But we need the actual value of updated rows. We figured out that the number of updated rows is returned inTableMutationOperator.javamethodgetOutput(). The updated rows number is passed tooperatorContextinstance and from there, all the way down to theQueryStatistics.javawhich make the number available in the event listeners.Additional context and related issues
In our ransomware defender platform, we need to monitor the behavior of users who have access to run queries on the DBs connected via Trino. So, we need to be notified of users' actions and the exact results of their actions. Trino returns the correct values for SELECT and INSERT queries. We need the same for UPDATE/DELTE as well.
The only issue is that the current pull request doesn't include the proper unit tests for the changes. Could you please help us with the proper way to write the unit tests?
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(X) Release notes are required, with the following suggested text:
#24596