Skip to content

Describe adding insert support in a connector#13046

Merged
raunaqmorarka merged 2 commits intotrinodb:masterfrom
nineinchnick:dev-guide-insert
Jul 27, 2022
Merged

Describe adding insert support in a connector#13046
raunaqmorarka merged 2 commits intotrinodb:masterfrom
nineinchnick:dev-guide-insert

Conversation

@nineinchnick
Copy link
Copy Markdown
Member

Description

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

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

How would you describe this change to a non-technical end user or system administrator?
n/a

Related issues, pull requests, and links

  • ConnectorPageSinkProvider is also briefly described in Dev guide connector features #12956, it could link to this page or maybe since it's short we should merge these two PRs. OTOH we might expand this more by adding examples on how to iterate over a Page, handle transactions, delayed writes, etc.

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:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Jun 30, 2022
@github-actions github-actions bot added the docs label Jun 30, 2022
@nineinchnick nineinchnick added the no-release-notes This pull request does not require release notes entry label Jul 18, 2022
@nineinchnick nineinchnick requested a review from mosabua July 26, 2022 11:14
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
connector specific information and will be passed to the page sink provider.
connector specific information, and it is passed to the page sink provider.

Copy link
Copy Markdown
Member Author

@nineinchnick nineinchnick Jul 26, 2022

Choose a reason for hiding this comment

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

I forgot about using the present tense, but I wanted to ask about passive voice - is it is passed passive and should we change it to say that the eng^H^H^H Trino passes it?

@nineinchnick
Copy link
Copy Markdown
Member Author

@mosabua ready for another round, I also ran the Vale linter on this and it found one extra space :D

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

One nit but essentially good to go.

@raunaqmorarka raunaqmorarka merged commit 2265ca8 into trinodb:master Jul 27, 2022
write deletions and/or updates to the underlying data store.
* The connector's ``UpdatablePageSource.getNextPage()`` implementation fetches the next page
from the underlying ``ConnectorPageSource``, optionally reformats the page, and returns it
from the underlying ``ConnectorPageSource``, optionally rebuild the page, and returns it
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rebuilds


When executing an ``INSERT`` statement, the engine calls the ``beginInsert()``
method in the connector, which receives a table handle and a list of columns.
It should return a ``ConnectorInsertTableHandle``, that can carry any
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The grammar of this sentence is off. cc @mosabua

@nineinchnick nineinchnick deleted the dev-guide-insert branch July 27, 2022 10:31
@github-actions github-actions bot added this to the 392 milestone Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed docs no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

5 participants