-
Notifications
You must be signed in to change notification settings - Fork 85
CrateDB: Add database destination adapter #237
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
base: main
Are you sure you want to change the base?
Conversation
dlt==1.10.0 | ||
dlt @ git+https://github.com/crate-workbench/dlt@cratedb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to see the patch landing and a subsequent release of the dlt package.
A corresponding notice on the dlt repository does not raise many expectations, it looks like CrateDB might be too late to the party.
📣 New destinations are unlikely to be merged due to high maintenance cost.
f86cf43
to
085f6a0
Compare
085f6a0
to
15cd568
Compare
> [!WARNING] | ||
> CrateDB supports the `replace` incremental materialization strategy, but | ||
> currently does not support the `delete+insert`, `merge`, or `scd2` strategies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hlcianfagna: I've blatantly copied this from the Clickhouse documentation, without knowing too much about corresponding details. As I think you have expertise in this area, could you please validate this statement in the context of ingestr and CrateDB, possibly including some hands-on validations? 🙏
At least I can tell from my basic orientation flights that I have been able to use Incremental Strategy: replace
successfully.
This is probably not a big surprise, as it doesn't need any database support at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this patch does not include much code at all, the topic is possibly a concern of the underlying dlt adapter.
However, we can also discuss relevant matters here, at your disposal, and also to create less noise on the dlt project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the ingestr incremental loading strategy is the same parameter that gets mapped to the dlt write disposition? In this case, the documentation about PostgreSQL's destination adapter says:
All write dispositions are supported.
So, after possibly validating all of them with CrateDB, we may want to adjust the documentation here.
[tool.hatch.metadata] | ||
allow-direct-references = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just temporary in order to support installing from a Git branch.
71eff57
to
e8abfcd
Compare
e8abfcd
to
e2b9ecf
Compare
About
The patch unlocks CrateDB as a database destination. Thanks!
Preview
Using this command, you can run the corresponding example without much ado.
Details
CrateDB's destination adapter is based on the dlt postgres adapter, so it is also using the psycopg2 package.
References