Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: fix missing table alias for on update column of image SQL #6075

Merged
merged 6 commits into from
Nov 26, 2023

Conversation

ptyin
Copy link
Member

@ptyin ptyin commented Nov 23, 2023

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

Add table alias to on update column when table alias is not blank

Ⅱ. Does this pull request fix one issue?

fixes #5488

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@ptyin ptyin changed the title bugfix: add table alias to on update column when table alias is not blank bugfix: fix missing table alias for on update column of image SQL Nov 23, 2023
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

Merging #6075 (0bf2467) into 2.x (960c298) will decrease coverage by 0.07%.
The diff coverage is 44.44%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6075      +/-   ##
============================================
- Coverage     49.60%   49.54%   -0.07%     
+ Complexity     4763     4757       -6     
============================================
  Files           908      908              
  Lines         31355    31362       +7     
  Branches       3776     3778       +2     
============================================
- Hits          15554    15537      -17     
- Misses        14273    14287      +14     
- Partials       1528     1538      +10     
Files Coverage Δ
.../rm/datasource/exec/BaseTransactionalExecutor.java 60.93% <44.44%> (+3.64%) ⬆️

... and 10 files with indirect coverage changes

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要考虑ONLY_CARE_UPDATE_COLUMNS为false的情况,以及多表更新需要识别后进行拦截sql,因为seata暂时没有支持联表更新的计划
You need to consider the situation where ONLY_CARE_UPDATE_COLUMNS is false, and intercept SQL statements for multi-table updates to identify and handle them, as Seata currently has no plans to support multi-table updates.

@ptyin
Copy link
Member Author

ptyin commented Nov 23, 2023

需要考虑ONLY_CARE_UPDATE_COLUMNS为false的情况,以及多表更新需要识别后进行拦截sql,因为seata暂时没有支持联表更新的计划 You need to consider the situation where ONLY_CARE_UPDATE_COLUMNS is false, and intercept SQL statements for multi-table updates to identify and handle them, as Seata currently has no plans to support multi-table updates.

From my POV, I think this PR should focus on addressing the issue #5488. The problems you mentioned is less related to the issue and can be supplemented in another PR.

@funky-eyes
Copy link
Contributor

需要考虑ONLY_CARE_UPDATE_COLUMNS为false的情况,以及多表更新需要识别后进行拦截sql,因为seata暂时没有支持联表更新的计划 You need to consider the situation where ONLY_CARE_UPDATE_COLUMNS is false, and intercept SQL statements for multi-table updates to identify and handle them, as Seata currently has no plans to support multi-table updates.

From my POV, I think this PR should focus on addressing the issue #5488. The problems you mentioned is less related to the issue and can be supplemented in another PR.

这个问题的本质是在sql包含多个库的情况下,列名没有指定别名,导致列查询不明确.那么在ONLY_CARE_UPDATE_COLUMNS等于false的情况下,也是存在这个问题的
The essence of the problem is that in the case where the sql contains multiple libraries, the column names are not specified as aliases, resulting in unclear column queries. This problem also exists when ONLY_CARE_UPDATE_COLUMNS is false.

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@funky-eyes funky-eyes added this to the 2.1.0 milestone Nov 26, 2023
@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. mode: AT AT transaction mode module/rm-datasource rm-datasource module labels Nov 26, 2023
@funky-eyes funky-eyes merged commit 7894473 into apache:2.x Nov 26, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mode: AT AT transaction mode module/rm-datasource rm-datasource module type: bug Category issues or prs related to bug.
Projects
None yet
2 participants