Initial version of extension to allow creating operators outside of duckdb core lib #5144

rjatwal · 2022-10-31T05:43:03Z

Sorry for the lack of description when posting this draft PR. Create an accompanying document which I will add shortly. Primarily the PR was opened as a discussion aid for a meeting later today.

Tishj · 2022-10-31T08:18:26Z

src/planner/binder.cpp

+		default: // LCOV_EXCL_START
+			auto &config = DBConfig::GetConfig(context);
+
+			for (auto &extension_op : config.operator_extensions) {


What is the reason to throw here, then catch it and perform the exact same code that resulted in the throw again?

The NotImplementedException is not of type StandardException so it should not be caught.

Sorry for the lack of description when posting this PR. Create an accompanying document which I will add shortly. Primarily the PR was opened as a discussion aid for a meeting later today.

Mytherin

Thanks for the PR! Looks good. Some comments:

Mytherin · 2022-10-31T10:24:05Z

src/planner/binder.cpp

+			                              StatementTypeToString(statement.type));
+		} // LCOV_EXCL_STOP
+	} catch (const StandardException &e) {
+		auto &config = DBConfig::GetConfig(context);


We generally try to avoid try/catch everywhere outside of at the "root" of query execution. Can this be removed? The extension should only throw an exception if there is an unrecoverable error (i.e. it failed to bind its own custom node) - in which case we want to propagate the exception upwards anyway.

The primary reason for adding the try-catch block was to call the extension's bind functions when duckdb Binding fails. Realize on second glance that using the existing exception block in

duckdb/src/planner/planner.cpp

Line 49 in c0a4ab9

if (ex.what() != string("Parameter Not Resolved Error: Parameter types could not be resolved")) {

is probably a better location.

Perhaps I'm misunderstanding but what is the use case for wanting to call the extensions' bind function when DuckDB's bind function fails? Isn't that what the call to the extensions' bind in the switch statement is for?

I will remove the call insight the switch as it is confusing and I don't have a good case for it. The call in the switch is only triggered by unhandled statement types. I can see this being useful if the flow of the parser extension changes, so that it returns a SQLStatement that is parsed by DuckDB. However, as it stands there should not be unhandled statement types.

With the try/catch I intend to capture catalog misses & other bind exceptions (StandardException - that fail a query, but don't kill a transaction). So

insert into remote_foo values(1,2) will trigger the call inside the catch if remote_foo is not a member of DuckDB's catalog.

Aha, that makes sense. I'm fine with that at least as a temporary solution - but I think a cleaner solution would be implementing the extensible catalog & pluggable storage systems (although clearly a lot more work). Are there other use cases you have for this - or is it limited to only emulating a pluggable catalog for now?

Mytherin · 2022-10-31T10:26:34Z

src/include/duckdb/planner/operator_extension.hpp

+public:
+	//! Creates a plan for a new logical operator injected into duckdb via the OperatorExtension's bind function or
+	//  the optimizer extension
+	create_plan_function_t CreatePlan;


Should this perhaps be implemented as virtual methods on the LogicalOperator that are only used for the extension operators? It seems a bit tricky to know which extension knows how to handle which logical operator otherwise.

Mytherin · 2022-11-09T15:38:38Z

@rj-atw could you still have a look at fixing the tidy check issue and mark this as ready for review?

rjatwal · 2022-11-11T09:24:58Z

@rj-atw could you still have a look at fixing the tidy check issue and mark this as ready for review?

@Mytherin Updated PR please take another look and merge if acceptable

Mytherin

Looks good to me - happy to merge it, but one question remains from my side:

Mytherin · 2022-11-11T12:13:12Z

src/planner/planner.cpp

+				this->names = bound_statement.names;
+				this->types = bound_statement.types;
+				this->plan = move(bound_statement.plan);
+				break;


Won't this still throw the exception - as this break only breaks out of the loop and into the throw statement?

Thanks for taking close look at my PR. Indeed I staged my commit incorrectly and forgot the guard check. Doing another round of testing and will push the update.

…uckdb core lib

…logical operator itself

…ner.cpp

Mytherin · 2022-11-12T13:30:34Z

Thanks for the updates!

* Use an inequality for the cardinality test to defend against zero LHS cardinalities fixes: duckdblabs/duckdb-internal#5144

Issue duckdb/duckdb#5144: AsOf Join Threshold (duckdb/duckdb#17978)

Issue duckdb/duckdb#5144: AsOf Join Threshold (duckdb/duckdb#17978) Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>

Issue duckdb/duckdb#5144: AsOf Join Threshold (duckdb/duckdb#17979) [CI] Skip some workflows when updating out of tree extensions SHA (duckdb/duckdb#17949)

Issue duckdb/duckdb#5144: AsOf Join Threshold (duckdb/duckdb#17979) [CI] Skip some workflows when updating out of tree extensions SHA (duckdb/duckdb#17949) Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>

Tishj reviewed Oct 31, 2022

View reviewed changes

Mytherin reviewed Oct 31, 2022

View reviewed changes

rjatwal force-pushed the rj/operator-extension-proposal branch from ca7b019 to 89c7a79 Compare November 10, 2022 11:42

rjatwal marked this pull request as ready for review November 10, 2022 23:46

Mytherin reviewed Nov 11, 2022

View reviewed changes

rjatwal added 7 commits November 11, 2022 10:15

Initial version of extension to allow creating operators outside of d…

77e11da

…uckdb core lib

Changing extension operators planning to use a virtual method on the …

bcd613d

…logical operator itself

Moved extension bind hook up the call stack from binder.cpp into plan…

3ab2d97

…ner.cpp

Removing forward-declaration made redundant by refactoring

e612716

Formatting

42ba867

Fixing ctidy check

b2c92e8

Adding if clause that I staged incorrectly

cc2eff0

rjatwal force-pushed the rj/operator-extension-proposal branch from 817b405 to cc2eff0 Compare November 11, 2022 18:29

Fixing edge case brought up by fuzzer

0bbd8b3

Mytherin merged commit 5fde799 into duckdb:master Nov 12, 2022

hawkfish added a commit to hawkfish/duckdb that referenced this pull request Jun 19, 2025

Issue duckdb#5144: AsOf Join Threshold

68df410

* Use an inequality for the cardinality test to defend against zero LHS cardinalities fixes: duckdblabs/duckdb-internal#5144

hawkfish added a commit to hawkfish/duckdb that referenced this pull request Jun 19, 2025

Issue duckdb#5144: AsOf Join Threshold

a5916b5

* Use an inequality for the cardinality test to defend against zero LHS cardinalities fixes: duckdblabs/duckdb-internal#5144

Mytherin added a commit that referenced this pull request Jun 20, 2025

Issue #5144: AsOf Join Threshold (#17978)

f11b826

* Use an inequality for the cardinality test to defend against zero LHS cardinalities fixes: duckdblabs/duckdb-internal#5144

Mytherin added a commit that referenced this pull request Jun 20, 2025

Issue #5144: AsOf Join Threshold (#17979)

cd376ee

* Use an inequality for the cardinality test to defend against zero LHS cardinalities fixes: duckdblabs/duckdb-internal#5144

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Jun 20, 2025

vendor: Update vendored sources to duckdb/duckdb@f11b826

8e4c14d

Issue duckdb/duckdb#5144: AsOf Join Threshold (duckdb/duckdb#17978)

github-actions bot mentioned this pull request Jun 20, 2025

vendor: Update vendored sources to duckdb/duckdb@f11b826ebeb32c1bc3d6f0d8c7e9a2eaeddc6ffb duckdb/duckdb-r#1255

Merged

github-actions bot mentioned this pull request Jun 22, 2025

vendor: Update vendored sources to duckdb/duckdb@cd376ee7f7b6fdc6dc8aa195058ec0c69a124725 duckdb/duckdb-r#1291

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Initial version of extension to allow creating operators outside of duckdb core lib #5144

Initial version of extension to allow creating operators outside of duckdb core lib #5144

Uh oh!

rjatwal commented Oct 31, 2022 •

edited

Loading

Uh oh!

Tishj Oct 31, 2022

Uh oh!

rjatwal Oct 31, 2022

Uh oh!

Mytherin left a comment

Uh oh!

Mytherin Oct 31, 2022

Uh oh!

rjatwal Oct 31, 2022

Uh oh!

Mytherin Oct 31, 2022

Uh oh!

rjatwal Oct 31, 2022 •

edited

Loading

Uh oh!

Mytherin Nov 1, 2022

Uh oh!

Mytherin Oct 31, 2022

Uh oh!

Mytherin commented Nov 9, 2022

Uh oh!

rjatwal commented Nov 11, 2022 •

edited

Loading

Uh oh!

Mytherin left a comment

Uh oh!

Mytherin Nov 11, 2022

Uh oh!

rjatwal Nov 11, 2022

Uh oh!

Mytherin commented Nov 12, 2022

Uh oh!

Uh oh!

Initial version of extension to allow creating operators outside of duckdb core lib #5144

Initial version of extension to allow creating operators outside of duckdb core lib #5144

Uh oh!

Conversation

rjatwal commented Oct 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mytherin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjatwal Oct 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mytherin commented Nov 9, 2022

Uh oh!

rjatwal commented Nov 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mytherin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mytherin commented Nov 12, 2022

Uh oh!

Uh oh!

rjatwal commented Oct 31, 2022 •

edited

Loading

rjatwal Oct 31, 2022 •

edited

Loading

rjatwal commented Nov 11, 2022 •

edited

Loading