feat(core): introduce dialect-specific function list and refactor BigQuery function lists#1366
Conversation
github-actions
bot
added
core
bigquery
ibis
rust
|
Warning Rate limit exceeded@goldmedal has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThreads an optional data_source through ibis-server, wren-core-py, and wren-core; gates remote-function registration for BigQuery; adds DataSource parsing/error; exposes a BigQuery dialect with many scalar/aggregate/window UDFs; updates UDF macros, ReturnType handling, and call-sites to accept data_source. Changes
Sequence Diagram(s)sequenceDiagram
participant ibis as ibis-server
participant wren_py as wren-core-py
participant wren_rs as wren-core (Rust)
participant df as DataFusion
ibis->>wren_py: PySessionContext::new(mdl_base64, remote_path, props, data_source)
wren_py->>wren_py: parse manifest (if present)
wren_py->>wren_py: resolve data_source (manifest preferred)
alt data_source == "bigquery"
wren_py->>wren_py: skip remote function registration
else
wren_py->>wren_py: read & register remote functions (CSV)
end
wren_py->>wren_rs: create_wren_ctx(Some(config), data_source)
wren_rs->>wren_rs: get_inner_dialect(data_source)
alt dialect present
wren_rs->>df: register dialect-supported scalar/aggregate/window UDFs
else
wren_rs->>df: register default function sets
end
wren_rs-->>wren_py: return SessionContext
wren_py-->>ibis: context initialized
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus during review:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
BigQuery has been tested locally |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ibis-server/app/mdl/rewriter.py (1)
159-169: Ensure consistency: rewrite_sync should also pass data_source.The async
rewritemethod passesdata_sourcetoget_session_context(line 149), butrewrite_syncdoes not (line 164-166). This inconsistency could lead to different behavior between sync and async execution paths.Apply this diff to align both methods:
def rewrite_sync( self, manifest_str: str, sql: str, properties: dict | None = None ) -> str: try: processed_properties = self.get_session_properties(properties) session_context = get_session_context( - manifest_str, self.function_path, processed_properties + manifest_str, + self.function_path, + processed_properties, + self.data_source.name if self.data_source else None, ) return session_context.transform_sql(sql) except Exception as e: raise WrenError(ErrorCode.INVALID_SQL, str(e), ErrorPhase.SQL_PLANNING)wren-core-py/src/context.rs (1)
90-117: Honor thedata_sourceargument when no manifest is supplied.When
mdl_base64isNoneyou currently discard the caller‑provideddata_source. That leaves us unable to bootstrap a dialect-aware context (e.g. BigQuery) without first constructing a manifest, and it also means the later BigQuery remote-function skip never triggers. Please thread the parseddata_sourcethrough this branch as well (convert the&strtoDataSourceand pass it intocreate_wren_ctx).
🧹 Nitpick comments (2)
wren-core/core/src/mdl/function/dialect/utils.rs (1)
3-5: Consider unifying with remote_function.rs's build_document.A similar
build_documentfunction exists inwren-core/core/src/mdl/function/remote_function.rs(lines 227-245) with richer parameter handling. If the two functions serve overlapping purposes, consider extracting a common builder utility to reduce duplication.wren-core/core/src/mdl/dialect/mod.rs (1)
20-20: Consider re-exports instead of exposing inner_dialect directly.Making
inner_dialectpublic exposes its internal implementation. If only specific items frominner_dialectneed to be public, consider usingpub usere-exports to maintain a cleaner API surface and preserve encapsulation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ibis-server/resources/function_list/bigquery.csvis excluded by!**/*.csv
📒 Files selected for processing (24)
ibis-server/app/config.py(1 hunks)ibis-server/app/mdl/core.py(1 hunks)ibis-server/app/mdl/rewriter.py(3 hunks)ibis-server/app/model/error.py(2 hunks)ibis-server/tools/query_local_run.py(1 hunks)wren-core-base/src/mdl/manifest.rs(6 hunks)wren-core-py/src/context.rs(3 hunks)wren-core-py/src/errors.rs(2 hunks)wren-core/core/src/mdl/dialect/inner_dialect.rs(5 hunks)wren-core/core/src/mdl/dialect/mod.rs(1 hunks)wren-core/core/src/mdl/function/dialect/bigquery/aggregate.rs(1 hunks)wren-core/core/src/mdl/function/dialect/bigquery/mod.rs(1 hunks)wren-core/core/src/mdl/function/dialect/bigquery/scalar.rs(1 hunks)wren-core/core/src/mdl/function/dialect/bigquery/window.rs(1 hunks)wren-core/core/src/mdl/function/dialect/mod.rs(1 hunks)wren-core/core/src/mdl/function/dialect/utils.rs(1 hunks)wren-core/core/src/mdl/function/macros.rs(2 hunks)wren-core/core/src/mdl/function/mod.rs(1 hunks)wren-core/core/src/mdl/function/remote_function.rs(16 hunks)wren-core/core/src/mdl/function/scalar/mod.rs(1 hunks)wren-core/core/src/mdl/mod.rs(79 hunks)wren-core/sqllogictest/src/test_context.rs(1 hunks)wren-core/wren-example/examples/plan-sql.rs(2 hunks)wren-core/wren-example/examples/to-many-calculation.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
wren-core/core/src/mdl/function/dialect/utils.rs (1)
wren-core/core/src/mdl/function/remote_function.rs (1)
build_document(228-246)
wren-core-py/src/errors.rs (1)
wren-core-base/src/mdl/manifest.rs (1)
new(110-114)
wren-core/core/src/mdl/function/dialect/bigquery/window.rs (2)
wren-core/core/src/mdl/function/dialect/utils.rs (1)
build_document(3-5)wren-core/core/src/mdl/function/remote_function.rs (4)
build_document(228-246)new(187-199)new(286-299)new(386-398)
wren-core/wren-example/examples/to-many-calculation.rs (1)
wren-core/core/src/mdl/mod.rs (1)
create_wren_ctx(368-396)
ibis-server/tools/query_local_run.py (1)
wren-core/core/src/mdl/mod.rs (1)
data_source(327-329)
wren-core/sqllogictest/src/test_context.rs (1)
wren-core/core/src/mdl/mod.rs (1)
create_wren_ctx(368-396)
wren-core-py/src/context.rs (4)
wren-core/core/src/mdl/mod.rs (6)
mdl(198-233)data_source(327-329)new(129-184)new(567-569)default(65-73)create_wren_ctx(368-396)wren-core-base/manifest-macro/src/lib.rs (2)
data_source(61-116)manifest(26-56)wren-core-base/src/mdl/manifest.rs (2)
new(110-114)from_str(158-183)wren-core-py/src/manifest.rs (1)
to_manifest(18-23)
ibis-server/app/mdl/core.py (3)
wren-core/core/src/mdl/mod.rs (1)
data_source(327-329)wren-core-base/manifest-macro/src/lib.rs (1)
data_source(61-116)wren-core-base/src/mdl/builder.rs (1)
data_source(90-93)
wren-core/wren-example/examples/plan-sql.rs (2)
wren-core/core/src/mdl/mod.rs (3)
mdl(198-233)create_wren_ctx(368-396)transform_sql_with_ctx(416-476)wren-core-base/manifest-macro/src/lib.rs (1)
manifest(26-56)
wren-core/core/src/mdl/function/remote_function.rs (1)
wren-core-py/src/context.rs (1)
new(82-226)
wren-core-base/src/mdl/manifest.rs (2)
wren-core-py/src/context.rs (1)
new(82-226)wren-core-base/src/mdl/builder.rs (7)
new(46-58)new(105-119)new(190-205)new(289-298)new(325-337)new(374-382)new(404-411)
wren-core/core/src/mdl/dialect/inner_dialect.rs (5)
wren-core/core/src/mdl/dialect/utils.rs (2)
scalar_function_to_sql_internal(51-76)args(30-48)wren-core/core/src/mdl/function/dialect/bigquery/mod.rs (3)
bigquery_aggregate_functions(300-338)bigquery_scalar_functions(73-298)bigquery_window_functions(340-356)wren-core/core/src/mdl/function/aggregate/mod.rs (1)
aggregate_functions(11-51)wren-core/core/src/mdl/function/scalar/mod.rs (1)
scalar_functions(18-192)wren-core/core/src/mdl/function/window/mod.rs (1)
window_functions(5-19)
ibis-server/app/mdl/rewriter.py (1)
ibis-server/app/model/data_source.py (1)
DataSource(60-214)
wren-core/core/src/mdl/mod.rs (5)
wren-core/core/src/mdl/dialect/inner_dialect.rs (1)
get_inner_dialect(112-121)wren-core-base/manifest-macro/src/lib.rs (2)
data_source(61-116)manifest(26-56)wren-core-py/src/context.rs (1)
new(82-226)wren-core/core/src/mdl/function/remote_function.rs (3)
new_with_return_type(201-211)new_with_return_type(317-328)new_with_return_type(400-410)wren-core/core/src/logical_plan/utils.rs (1)
try_map_data_type(102-107)
wren-core/core/src/mdl/function/dialect/bigquery/aggregate.rs (2)
wren-core/core/src/mdl/function/remote_function.rs (8)
build_document(228-246)new(187-199)new(286-299)new(386-398)from(215-225)from(332-342)from(414-423)new_with_alias(301-315)wren-core/core/src/mdl/function/dialect/utils.rs (1)
build_document(3-5)
wren-core/core/src/mdl/function/dialect/bigquery/scalar.rs (2)
wren-core/core/src/mdl/function/remote_function.rs (4)
build_document(228-246)new(187-199)new(286-299)new(386-398)wren-core/core/src/mdl/function/dialect/utils.rs (1)
build_document(3-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: cargo check
- GitHub Check: ci
🔇 Additional comments (19)
ibis-server/app/config.py (1)
64-67: LGTM! BigQuery function list handled in Wren Core.The early return for BigQuery is correct and aligns with the PR's introduction of dialect-specific function lists in Wren Core.
ibis-server/app/mdl/rewriter.py (1)
45-47: LGTM! Data source awareness added to embedded rewriter.The change correctly threads
data_sourcethrough toEmbeddedEngineRewriterfor dialect-aware context creation.ibis-server/app/model/error.py (1)
8-8: LGTM! Updated to correct Starlette constant.The change from
HTTP_422_UNPROCESSABLE_ENTITYtoHTTP_422_UNPROCESSABLE_CONTENTaligns with modern Starlette conventions.wren-core-py/src/errors.rs (1)
92-96: LGTM! Error conversion follows established patterns.The
From<ParsedDataSourceError>implementation is consistent with other error conversions in this file and properly integrates the new data source error type.wren-core/core/src/mdl/function/dialect/mod.rs (1)
1-2: LGTM! Clean module organization.The public
bigquerymodule and privateutilsmodule are appropriately scoped for the dialect-specific function list feature.wren-core/core/src/mdl/function/scalar/mod.rs (1)
12-12: LGTM! Macro rename improves clarity.The rename from
make_udf_functiontomake_datafusion_udf_functionmakes the DataFusion dependency explicit and improves code readability.wren-core/wren-example/examples/to-many-calculation.rs (1)
20-20: LGTM: API update aligns with new data_source parameter.The addition of the second
Noneparameter correctly adapts to the updatedcreate_wren_ctxsignature, which now accepts an optional data_source.wren-core/core/src/mdl/function/mod.rs (1)
2-2: LGTM: Public dialect module exposure.The addition of
pub mod dialect;appropriately exposes dialect-specific function registries for external access, supporting the BigQuery dialect implementation.ibis-server/tools/query_local_run.py (1)
80-80: LGTM: data_source parameter threaded to SessionContext.The addition of the
data_sourceargument properly threads the data source context from the environment variable to the session initialization, enabling dialect-aware behavior.wren-core/sqllogictest/src/test_context.rs (1)
66-66: LGTM: Test context updated for new API.The addition of
Noneas the second parameter correctly adapts the test harness to the updatedcreate_wren_ctxsignature, maintaining default (non-dialect-specific) behavior for tests.ibis-server/app/mdl/core.py (1)
7-15: LGTM: Public API extended with data_source parameter.The addition of the optional
data_sourceparameter toget_session_contextproperly extends the public API while maintaining backward compatibility through the defaultNonevalue. The parameter is correctly threaded to the underlyingwren_core.SessionContext.wren-core/wren-example/examples/plan-sql.rs (2)
7-8: LGTM: Updated imports for dialect support.The addition of
DataSourceandcreate_wren_ctximports correctly supports the dialect-aware context creation demonstrated in this example.
22-22: LGTM: Demonstrates BigQuery dialect usage.The explicit use of
Some(&DataSource::BigQuery)in the context creation effectively demonstrates the new dialect-aware functionality, ensuring BigQuery-specific UDFs are registered.wren-core/core/src/mdl/function/macros.rs (4)
6-21: LGTM: Renamed macro for clarity.Renaming the original macro to
make_datafusion_udf_functionclearly distinguishes it from the new expression-basedmake_udf_functionmacro, preventing confusion between the type-based (<$UDF>::new()) and expression-based approaches.
23-39: LGTM: Expression-based UDF macro added.The new
make_udf_functionmacro appropriately accepts a pre-constructed UDF expression ($UDF:expr) and wraps it in a singleton pattern. This enables more flexible UDF registration, particularly for dialect-specific functions.
41-57: LGTM: Expression-based UDAF macro added.The
make_udaf_functionmacro follows the same pattern asmake_udf_function, providing consistent expression-based aggregate function registration.
59-75: LGTM: Expression-based UDWF macro added.The
make_udwf_functionmacro completes the set of expression-based macros, providing consistent window function registration following the same singleton pattern.wren-core/core/src/mdl/function/dialect/bigquery/window.rs (2)
8-19: LGTM: percentile_cont window function definition.The
percentile_contfunction is correctly defined with:
ReturnType::SameAsInput(appropriate for continuous percentiles)Signature::any(2, Volatility::Immutable)(correct for 2-argument percentile function)- Clear documentation with SQL example
21-32: LGTM: percentile_disc window function definition.The
percentile_discfunction is correctly defined with:
ReturnType::SameAsInput(appropriate for discrete percentiles)Signature::any(2, Volatility::Immutable)(correct for 2-argument percentile function)- Clear documentation with SQL example
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wren-core-py/src/context.rs (1)
116-132: Catch disagreements between manifest and explicit data_source.Right now we silently prefer the manifest value when both sources are present. If callers accidentally pass a conflicting
data_source, they won’t discover the mismatch until much later. Please fail fast in that case so we surface the configuration error immediately.You can parse the optional argument up front, compare, and only proceed when they agree:
- // If the manifest has a data source, use it. - // Otherwise, if the data_source parameter is provided, use it. - // Otherwise, use None. - let data_source = if let Some(ds) = &manifest.data_source { - Some(*ds) - } else if let Some(ds_str) = data_source { - Some(DataSource::from_str(ds_str).map_err(CoreError::from)?) - } else { - None - }; + let param_data_source = data_source + .map(|ds| DataSource::from_str(ds).map_err(CoreError::from)) + .transpose()?; + + if let (Some(manifest_ds), Some(param_ds)) = (manifest.data_source, param_data_source) { + if manifest_ds != param_ds { + return Err(CoreError::new( + format!( + "Manifest data_source '{manifest_ds:?}' conflicts with explicit data_source '{param_ds:?}'" + ) + .as_str(), + ) + .into()); + } + } + + // If the manifest has a data source, use it; otherwise fall back to the parsed argument. + let data_source = manifest.data_source.or(param_data_source);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
wren-core-py/src/context.rs(4 hunks)wren-core-py/tests/test_modeling_core.py(2 hunks)wren-core/core/src/mdl/function/dialect/bigquery/aggregate.rs(1 hunks)wren-core/core/src/mdl/mod.rs(79 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
wren-core/core/src/mdl/mod.rs (3)
wren-core/core/src/mdl/dialect/inner_dialect.rs (1)
get_inner_dialect(112-121)wren-core-py/src/context.rs (1)
new(82-208)wren-core/core/src/mdl/function/remote_function.rs (3)
new_with_return_type(201-211)new_with_return_type(317-328)new_with_return_type(400-410)
wren-core-py/src/context.rs (6)
wren-core/core/src/mdl/mod.rs (7)
mdl(198-233)data_source(327-329)new(129-184)new(567-569)default(65-73)create_wren_ctx(368-396)register_remote_function(529-554)wren-core-py/src/extractor.rs (2)
mdl_base64(174-234)new(22-29)wren-core-base/manifest-macro/src/lib.rs (2)
data_source(61-116)manifest(26-56)wren-core-py/src/errors.rs (12)
new(18-22)from(26-28)from(32-34)from(38-40)from(44-46)from(50-52)from(56-65)from(69-71)from(75-77)from(81-83)from(87-89)from(93-95)wren-core-base/src/mdl/manifest.rs (2)
new(110-114)from_str(158-183)wren-core-py/src/manifest.rs (1)
to_manifest(18-23)
wren-core-py/tests/test_modeling_core.py (4)
wren-core/core/src/mdl/mod.rs (1)
data_source(327-329)wren-core-base/manifest-macro/src/lib.rs (1)
data_source(61-116)wren-core-base/src/mdl/py_method.rs (1)
data_source(49-51)wren-core-base/src/mdl/builder.rs (1)
data_source(90-93)
wren-core/core/src/mdl/function/dialect/bigquery/aggregate.rs (2)
wren-core/core/src/mdl/function/remote_function.rs (8)
build_document(228-246)new(187-199)new(286-299)new(386-398)from(215-225)from(332-342)from(414-423)new_with_alias(301-315)wren-core/core/src/mdl/function/dialect/utils.rs (1)
build_document(3-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: ci
- GitHub Check: test
- GitHub Check: cargo check
🔇 Additional comments (4)
wren-core/core/src/mdl/function/dialect/bigquery/aggregate.rs (2)
93-104: Past issue resolved:array_concat_aggreturn type is now correct.The previous review flagged that
array_concat_aggwas incorrectly usingReturnType::ArrayOfInputFirstArgument, which would double-wrap array types. The function now correctly usesReturnType::SameAsInput, ensuring that input arrays maintain their type structure (e.g.,ARRAY<INT64>staysARRAY<INT64>rather than becomingARRAY<ARRAY<INT64>>).
1-209: Well-structured BigQuery aggregate function definitions.The implementation is solid:
- All 14 UDAFs have correct return types matching BigQuery semantics
- Signatures appropriately balance flexibility (
any(n)) with type safety (coercible signatures forlogical_and/logical_or)- Consistent, clear documentation with SQL examples for each function
- Proper handling of the deprecated
group_concatlegacy function- The
countifalias registration follows conventionwren-core-py/tests/test_modeling_core.py (1)
20-21: DataFusion fixture keeps the coverage relevant.Switching the shared manifest to
datafusionmeans the CSV-driven remote-function tests still exercise the registration path now that BigQuery relies on the dialect registry. Nice catch.wren-core/core/src/mdl/mod.rs (1)
369-387: Dialect-aware registration looks solid.Passing the manifest’s data source into
create_wren_ctxand letting the inner dialect supply the UDF registries keeps the BigQuery lists isolated without disturbing the generic fallback.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ibis-server/wren/session/__init__.py (1)
29-31: Constructor call missing data_source argument for consistency.The
EmbeddedEngineRewriterconstructor now accepts an optionaldata_sourceparameter (seerewriter.py:135-136), and theContexthasself.data_sourceavailable. However, this constructor call doesn't pass it, relying instead on passingdata_sourceas a parameter torewrite_syncat line 100.This is related to the inconsistency flagged in
rewriter.py. For consistency with theRewriterusage pattern (which passesdata_sourceto the constructor), consider updating this call.Apply this diff:
self.rewriter = EmbeddedEngineRewriter( - get_config().get_remote_function_list_path(data_source) + get_config().get_remote_function_list_path(data_source), + data_source )Then update line 100 to remove the parameter (assuming
rewrite_syncis refactored per the previous comment):self.planned_sql = self.context.rewriter.rewrite_sync( - self.manifest, self.wren_sql, self.properties, self.context.data_source + self.manifest, self.wren_sql, self.properties )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ibis-server/app/mdl/rewriter.py(4 hunks)ibis-server/wren/session/__init__.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
ibis-server/wren/session/__init__.py (1)
wren-core-base/manifest-macro/src/lib.rs (1)
manifest(26-56)
ibis-server/app/mdl/rewriter.py (2)
ibis-server/app/model/data_source.py (1)
DataSource(60-214)ibis-server/app/mdl/core.py (1)
get_session_context(7-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
wren-core/core/src/mdl/dialect/inner_dialect.rs (1)
98-111: Consider adding documentation for the new trait methods.The implementation correctly provides default methods for dialect-specific function registration, which aligns well with the PR objectives. However, adding doc comments would improve maintainability by explaining when and how these methods should be overridden.
Example documentation:
/// Define the supported UDFs for the dialect which will be registered in the execution context. /// /// By default, returns the standard DataFusion scalar functions. Override this method /// to provide dialect-specific scalar functions. fn supported_udfs(&self) -> Vec<Arc<datafusion::logical_expr::ScalarUDF>> { scalar_functions() } /// Define the supported UDAFs for the dialect which will be registered in the execution context. /// /// By default, returns the standard DataFusion aggregate functions. Override this method /// to provide dialect-specific aggregate functions. fn supported_udafs(&self) -> Vec<Arc<datafusion::logical_expr::AggregateUDF>> { aggregate_functions() } /// Define the supported UDWFs for the dialect which will be registered in the execution context. /// /// By default, returns the standard DataFusion window functions. Override this method /// to provide dialect-specific window functions. fn supported_udwfs(&self) -> Vec<Arc<datafusion::logical_expr::WindowUDF>> { window_functions() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ibis-server/app/mdl/rewriter.py(4 hunks)ibis-server/wren/session/__init__.py(1 hunks)wren-core/core/src/mdl/dialect/inner_dialect.rs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ibis-server/app/mdl/rewriter.py
🧰 Additional context used
🧬 Code graph analysis (1)
wren-core/core/src/mdl/dialect/inner_dialect.rs (5)
wren-core/core/src/mdl/dialect/utils.rs (2)
scalar_function_to_sql_internal(51-76)args(30-48)wren-core/core/src/mdl/function/dialect/bigquery/mod.rs (3)
bigquery_aggregate_functions(300-338)bigquery_scalar_functions(73-298)bigquery_window_functions(340-356)wren-core/core/src/mdl/function/aggregate/mod.rs (1)
aggregate_functions(11-51)wren-core/core/src/mdl/function/scalar/mod.rs (1)
scalar_functions(18-192)wren-core/core/src/mdl/function/window/mod.rs (1)
window_functions(5-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: ci
- GitHub Check: cargo check
🔇 Additional comments (4)
ibis-server/wren/session/__init__.py (1)
99-103: LGTM! Formatting improvement enhances readability.The multi-line formatting of the
rewrite_synccall improves code readability while preserving the original semantics.wren-core/core/src/mdl/dialect/inner_dialect.rs (3)
20-21: LGTM!The
Arcimport is necessary for the new trait method return types.
23-26: LGTM!The imports correctly bring in both BigQuery-specific and default function lists needed for the new dialect-specific registration mechanism.
152-162: LGTM!The BigQuery dialect correctly overrides the new methods to provide BigQuery-specific function lists, enabling proper function registration for BigQuery data sources.
|
The Oracle test is flaky in github flow. I have tested it locally |
|
Thanks @goldmedal |
…Query function lists (Canner#1366)
2 participants
Description
To provide accurate functions, this PR redefines the function list by the signature of DataFusion. Most functions could have multiple signatures; We defined the function lists by a simple record of csv. It isn't enough to define multiple signatures.
The
InnerDialecthas 3 methods for the supported functions, which will be registered when the session context is createdWe can define the supported function for each data source. If the list isn't defined, we register the default function list.
BigQuery Function list
wren-core/core/src/mdl/function/dialect/bigquery.ibis-server/resources/white_function_list/bigquery.csvSummary by CodeRabbit
New Features
Bug Fixes
Improvements