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

Improve JDBC connector SQL support section #24523

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mosabua
Copy link
Member

@mosabua mosabua commented Dec 18, 2024

Description

  • First improve the MERGE limitation and make it a fragment
  • Move the limitations into SQL support section, add headers, and improve overview list

For now this is PostgresSQL connector only. Once I get some feedback that this is good .. I will expand the second commit and do the same changes for all other JDBC connectors.

Additional context and related issues

Follow up to #23034 and related PRs.

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

As applicable for PostgreSQL connector for now. Also extract into
a fragment so it can be reused in other connectors.
@mosabua mosabua requested a review from chenjian2664 December 18, 2024 23:04
@cla-bot cla-bot bot added the cla-signed label Dec 18, 2024
@mosabua mosabua requested a review from ebyhr December 18, 2024 23:04
@github-actions github-actions bot added the docs label Dec 18, 2024
@mosabua mosabua requested a review from electrum December 18, 2024 23:04
The connector supports adding rows using [MERGE statements](/sql/merge), if the
`merge.non-transactional-merge.enabled` catalog property or the corresponding
`non_transactional_merge_enabled` catalog session property is set to
`true`. Merge is only supported for directly modifying target tables.
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure this sentence makes sense and takes the original content and captures what was meant @chenjian2664 and @ebyhr

Copy link
Contributor

@chenjian2664 chenjian2664 left a comment

Choose a reason for hiding this comment

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

Thanks. I was also thought to do this, but forgot :)
Involve @kokosing

@@ -0,0 +1,10 @@
### Non-transactional MERGE

The connector supports adding rows using [MERGE statements](/sql/merge), if the
Copy link
Contributor

Choose a reason for hiding this comment

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

The connector supports adding rows using MERGE can add/update/delete rows, maybe you have better phrase? like change adding rows to modifying rows?

@chenjian2664
Copy link
Contributor

Phoenix Connector also supports non transactional merge @mosabua

@mosabua
Copy link
Member Author

mosabua commented Dec 24, 2024

Ignite connector also needs to be updated

- Consistent wording
_ Markdown link syntax
- Move related configs to SQL support section
- Improve list and links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants