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

fix(http): make HTTP text import for WAL tables to write via WAL #3648

Merged
merged 9 commits into from
Aug 15, 2023

Conversation

ideoma
Copy link
Collaborator

@ideoma ideoma commented Aug 14, 2023

REST imp endpoint was not updated to write to WAL tables using WAL infrastructure. This PR makes the writing to behave as expected. Also fixes #3632

@ideoma
Copy link
Collaborator Author

ideoma commented Aug 14, 2023

[PR Coverage check]

😍 pass : 70 / 73 (95.89%)

file detail

path covered line new line coverage
🔵 io/questdb/cairo/TableUtils.java 11 14 78.57%
🔵 io/questdb/cairo/TableReaderMetadata.java 1 1 100.00%
🔵 io/questdb/cutlass/text/CairoTextWriter.java 24 24 100.00%
🔵 io/questdb/cairo/pool/MetadataPool.java 17 17 100.00%
🔵 io/questdb/cairo/DynamicTableReaderMetadata.java 17 17 100.00%

@puzpuzpuz puzpuzpuz self-requested a review August 15, 2023 08:55
@puzpuzpuz
Copy link
Contributor

Looks like as a side effect of this change, the serial mode of SQL COPY also started writing to WAL. Probably, it's worth it to add some tests around it.

As for the parallel mode where we require the table to be empty, we should at least document that it writes to the end table instead of WAL and/or create a separate GH issue.

WDYT?

@puzpuzpuz
Copy link
Contributor

Looks like as a side effect of this change, the serial mode of SQL COPY also started writing to WAL. Probably, it's worth it to add some tests around it.

Hmm, I forgot that it's used for non-partitioned tables, so it's not applicable to WAL tables. On WAL tables COPY should be parallel always.

@ideoma ideoma merged commit 1aacde5 into master Aug 15, 2023
21 checks passed
@ideoma ideoma deleted the fix-wal-text-imp branch August 15, 2023 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deduplication doesn't apply to /imp CSV upload
4 participants