Skip to content

Describe record cursor type mapping in dev guide#12976

Merged
martint merged 1 commit intotrinodb:masterfrom
nineinchnick:dev-guide-record-sets
Aug 22, 2022
Merged

Describe record cursor type mapping in dev guide#12976
martint merged 1 commit intotrinodb:masterfrom
nineinchnick:dev-guide-record-sets

Conversation

@nineinchnick
Copy link
Copy Markdown
Member

Description

Describe how to map types when implementing a RecordCursor, like how to encode strings into slices or pack timestamps. Add some basic examples of creating blocks.

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

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 24, 2022
@nineinchnick nineinchnick added no-release-notes This pull request does not require release notes entry docs labels Jun 24, 2022
Copy link
Copy Markdown
Member

@colebow colebow left a comment

Choose a reason for hiding this comment

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

This overall looks good to me, though I definitely don't know enough about the subject matter to review for correctness / accuracy.

@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch from 6637faf to d0ccd07 Compare June 29, 2022 07:17
@MiguelWeezardo MiguelWeezardo requested review from MiguelWeezardo and removed request for MiguelWeezardo July 5, 2022 12:21
@MiguelWeezardo
Copy link
Copy Markdown
Member

Looks good

@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch 3 times, most recently from 4f6ba91 to f2ca2b1 Compare July 24, 2022 10:22
@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch 3 times, most recently from 902d4b3 to 6b0bbe3 Compare July 24, 2022 18:34
@nineinchnick
Copy link
Copy Markdown
Member Author

@martint PTAL

@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch from 6b0bbe3 to 57e322c Compare July 27, 2022 08:55
@nineinchnick
Copy link
Copy Markdown
Member Author

@martint AC, PTAL, and thanks so much!

@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch from 57e322c to c02cb9f Compare July 27, 2022 09:19
@nineinchnick
Copy link
Copy Markdown
Member Author

@martint PTAL, there are two comments I think I addressed but need your confirmation
@mosabua PTAL generally (or maybe you know others who could review the wording)

@colebow colebow requested a review from jhlodin August 2, 2022 20:33
Copy link
Copy Markdown
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.

Some suggested style and formatting edits

@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch from c02cb9f to 49c7153 Compare August 3, 2022 15:02
@nineinchnick
Copy link
Copy Markdown
Member Author

@martint PTAL

@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch from 49c7153 to 218a5bd Compare August 22, 2022 07:35
@nineinchnick
Copy link
Copy Markdown
Member Author

@martint there are still two unresolved comments where I asked for clarification, PTAL.

@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch from 218a5bd to 6056706 Compare August 22, 2022 18:15
@nineinchnick nineinchnick force-pushed the dev-guide-record-sets branch from 6056706 to 7a9b0f2 Compare August 22, 2022 18:18
@nineinchnick
Copy link
Copy Markdown
Member Author

@martint all done!

@martint martint merged commit 2f62586 into trinodb:master Aug 22, 2022
@github-actions github-actions bot added this to the 394 milestone Aug 22, 2022
@nineinchnick nineinchnick deleted the dev-guide-record-sets branch August 24, 2022 09:16
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.

6 participants