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.

Sign up for GitHub

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

Jump to bottom

Remove SQL data management url and add link to insert #11674

Merged
merged 1 commit into from
Mar 28, 2022
Merged

Remove SQL data management url and add link to insert #11674

merged 1 commit into from
Mar 28, 2022

Conversation

rosewms
Copy link
Contributor

@rosewms rosewms commented Mar 25, 2022 •
edited
Loading

Description

  • Remove url for Data management
  • Add a specific url for INSERT

Is this change a fix, improvement, new feature, refactoring, or other? Fix/improvement

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific) Memory connector

How would you describe this change to a non-technical end user or system administrator? Specifying which data management commands are supported.

Related issues, pull requests, and links

  • Error was reported in the Slack troubleshooting channel, here's the thread with the info.

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Mar 25, 2022
@rosewms rosewms requested review from jhlodin and ebyhr March 25, 2022 18:53
@rosewms rosewms added the docs label Mar 25, 2022
@rosewms rosewms requested a review from mosabua March 25, 2022 18:54
jhlodin
jhlodin approved these changes Mar 25, 2022
View reviewed changes
Copy link
Contributor

@jhlodin jhlodin left a comment

Choose a reason for hiding this comment

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

LGTM % ordering nit

docs/src/main/sphinx/connector/memory.rst Outdated Show resolved Hide resolved
@rosewms rosewms force-pushed the rw-memory-connector branch from f6c8353 to 5eb7b8a Compare March 25, 2022 19:52
@mosabua
Copy link
Member

mosabua commented Mar 25, 2022 •
edited
Loading

The real reason is that UPDATE is not supported... but that is in the list for data management.

ebyhr
ebyhr approved these changes Mar 26, 2022
View reviewed changes
Copy link
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Looks good except for a UPDATE. Please update the commit & PR title when you address the comment.

docs/src/main/sphinx/connector/memory.rst Outdated Show resolved Hide resolved
@rosewms rosewms force-pushed the rw-memory-connector branch from 5eb7b8a to ab74ce7 Compare March 28, 2022 15:51
@rosewms rosewms changed the title Remove SQL data management url and add links to insert and update Remove SQL data management url and add links to insert Mar 28, 2022
@rosewms rosewms changed the title Remove SQL data management url and add links to insert Remove SQL data management url and add link to insert Mar 28, 2022
@rosewms
Copy link
Contributor Author

rosewms commented Mar 28, 2022

@ebyhr thank you for your help, I made the requested changes so this PR should be ready to merge.

@ebyhr ebyhr merged commit d0b4c44 into trinodb:master Mar 28, 2022
@ebyhr
Copy link
Member

ebyhr commented Mar 28, 2022

Merged, thanks!

@github-actions github-actions bot added this to the 376 milestone Mar 28, 2022
@mosabua mosabua mentioned this pull request Mar 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr approved these changes

@jhlodin jhlodin jhlodin approved these changes

@mosabua mosabua mosabua approved these changes

Assignees
No one assigned
Labels
cla-signed docs
Milestone
376
Development

Successfully merging this pull request may close these issues.
4 participants
@rosewms @mosabua @ebyhr @jhlodin