MehulBatra commented on PR #2525:
URL: https://github.com/apache/fluss/pull/2525#issuecomment-3828043369

   > Thank @MehulBatra for the great pull request. I left some comments inline.
   > 
   > Besides, regarding the metadata column `_change_type` in the `$changelog` 
and `$binlog` virtual tables, the current design uses abbreviations like `+I`, 
`-U`, `+U`, and `-D`.
   > 
   > I re-considered it again, while these align with Flink's internal 
`RowKind` representation, I think maybe using full descriptive strings instead 
would be better, such as:
   > 
   > * `insert` (instead of `+I` and `+A`)
   > * `update_before` (instead of `-U`)
   > * `update_after` (instead of `+U`)
   > * `delete` (instead of `-D`)
   > 
   > And for `$binlog` tables
   > 
   > * `insert` (instead of `I`)
   > * `update` (instead of `U`)
   > * `delete` (instead of `D`)
   > 
   > Reasoning:
   > 
   > * **Improved Readability**: Full words are much more intuitive for SQL 
users, especially those who may not be familiar with Flink's internal row-kind 
symbols.
   > * **Better User Experience**: It makes the audit and incremental ETL 
queries more self-documenting (e.g., `WHERE _change_type = 'insert'`).
   > * **Consistency**: This approach is widely adopted by other query engines, 
like databricks (uses `insert`, `delete`, `update_preimage`), trino (uses 
`insert` and `delete`), snowflake (uses `INSERT`, `DELETE`)
   > 
   > What do you think about this change? This requires to change the previous 
`$changelog` table implementation, but I think it is worth to do this before 
the release.
   > 
   > [1] databricks: 
https://docs.databricks.com/aws/en/sql/language-manual/functions/table_changes 
[2] trino: https://trino.io/docs/current/connector/iceberg.html#table-changes 
[3] snowflake: 
https://docs.snowflake.com/en/user-guide/streams-intro#label-streams-changes-clause
   
   Agreed. Following industry standards (Databricks, Trino, Snowflake) makes 
integration and migration much easier for users. The improved readability is 
worth the change before release.
   I'll update the this PR accordingly and create a seperate one for changelog.


-- 
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]

Reply via email to