add new field updatedRows to QueryStatistics.java
#24808
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued. add method `recordUpdatedPositions` to the OperatorContext class update the updatedPositions for the queries run through MergeWriterOperator.java
This brings updated timezonedb to version 2024b (openjdk/jdk23u@73b2341) which amends historical timezone definitions for Mexico/Bahia_Banderas that we use for testing timezone gap around the unix timestamp epoch. Corresponding Joda time update also has these timezone definitions updated. PostgreSQL test server was upgraded to 12 to correctly handle UTC around epoch. MySQL was updated to 8.0.41 due to the same reason. Temporary downgrade JDK for ppc64 to make CI happy
…e in EventListener callbacks. It comes from TableMutationOperator.java where update/delete queries are issued. add method `recordUpdatedPositions` to the OperatorContext class update the updatedPositions for the queries run through MergeWriterOperator.java
….com/bhzaeri/trino-updated-rows into event-listener-updated-rows-number-3
|
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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()and also inMergeWriterOperator.javathe same method. 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.So far, we have tested this successfully on SQL Serevr, Hive, Mysql, and Postgresql.
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