feat(core): support to_char format numeric values#1349
feat(core): support to_char format numeric values#1349douenergy merged 3 commits intoCanner:mainfrom
to_char format numeric values#1349Conversation
github-actions
bot
added
core
ibis
rust
WalkthroughThis pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant Test as Test/Query
participant Engine as Wren Engine
participant UDF as to_char UDF
participant DataFusion as DataFusion
Test->>Engine: SQL with to_char(column, pattern)
Engine->>UDF: Resolve to_char function
activate UDF
Note over UDF: LazyLock<Arc<ScalarUDF>><br/>initialized on first call
UDF-->>Engine: ScalarUDF instance
deactivate UDF
Engine->>DataFusion: Register & execute scalar UDF
DataFusion->>DataFusion: invoke_with_args<br/>(type coercion & formatting)
DataFusion-->>Engine: Formatted UTF8 result
Engine-->>Test: Query result with formatted values
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Multiple interconnected files with new macro infrastructure and UDF implementation following established patterns. Complexity stems from macro mechanics, DataFusion type signature configuration, and cross-module integration; offset by consistent pattern reuse and comprehensive test validation. Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wren-core-py/src/context.rs (2)
344-361: Potential panic on NULLs and unwrap; make row decoding null‑safedescription and function_type may be NULL; calling value(row) and unwrap() can panic. Handle nulls and unknown function types gracefully.
Apply this diff:
@@ - for batch in batches { - let name_array = batch.column(0).as_string::<i32>(); - let function_type_array = batch.column(1).as_string::<i32>(); - let description_array = batch.column(2).as_string::<i32>(); - - for row in 0..batch.num_rows() { - let name = name_array.value(row).to_string(); - let description = description_array.value(row).to_string(); - let function_type = function_type_array.value(row).to_string(); - - functions.push(RemoteFunctionDto { - name, - description: Some(description), - function_type: FunctionType::from_str(&function_type).unwrap(), - }); - } - } + for batch in batches { + let name_array = batch.column(0).as_string::<i32>(); + let function_type_array = batch.column(1).as_string::<i32>(); + let description_array = batch.column(2).as_string::<i32>(); + + for row in 0..batch.num_rows() { + // name is mandatory; skip if NULL + if !name_array.is_valid(row) { + continue; + } + let name = name_array.value(row).to_string(); + + // description is optional + let description = if description_array.is_valid(row) { + Some(description_array.value(row).to_string()) + } else { + None + }; + + // function_type may be NULL or unknown; default to Scalar or skip unknowns + let function_type_str = if function_type_array.is_valid(row) { + function_type_array.value(row).to_string() + } else { + "scalar".to_string() + }; + let function_type = match FunctionType::from_str(&function_type_str) { + Ok(ft) => ft, + Err(_) => continue, // skip unrecognized types + }; + + functions.push(RemoteFunctionDto { + name, + description, + function_type, + }); + } + }
327-342: Usefunction_nameinstead ofroutine_namein SQL query
DataFusion 50’s information_schema.routines definesfunction_name(notroutine_name), so update the SELECT clause in wren-core-py/src/context.rs to user.function_name.
🧹 Nitpick comments (10)
wren-core/core/src/mdl/function/scalar/to_char.rs (1)
29-40: Review signature precision for Decimal types.Lines 38-39 specify exact
Decimal128(38, 10)andDecimal256(38, 10)signatures. These fixed precision/scale values may not match the actual decimal types in user data, potentially causing type coercion failures. Consider usingTypeSignature::Coerciblefor decimal types as well, or document why these specific precision/scale values are required.wren-core/core/src/mdl/function/scalar/mod.rs (1)
145-191: Unify nested function paths for readability (optional).Mixed use of
datafusion::functions_nested::...and bare module names (string::,range::, etc.). Prefer one style consistently (sincefunctions_nested::*is imported). This is purely cosmetic.Apply this diff to normalize the fully-qualified occurrences:
- datafusion::functions_nested::concat::array_append_udf(), - datafusion::functions_nested::concat::array_prepend_udf(), - datafusion::functions_nested::concat::array_concat_udf(), + concat::array_append_udf(), + concat::array_prepend_udf(), + concat::array_concat_udf(), @@ - datafusion::functions_nested::repeat::array_repeat_udf(), + repeat::array_repeat_udf(), @@ - datafusion::functions_nested::reverse::array_reverse_udf(), + reverse::array_reverse_udf(), @@ - datafusion::functions_nested::replace::array_replace_n_udf(), - datafusion::functions_nested::replace::array_replace_all_udf(), - datafusion::functions_nested::replace::array_replace_udf(), + replace::array_replace_n_udf(), + replace::array_replace_all_udf(), + replace::array_replace_udf(),Also applies to: 152-155, 172-175, 183-185
wren-core/core/src/mdl/function/macros.rs (3)
11-19: Confirm MSRV forstd::sync::LazyLockor switch toonce_cell::sync::Lazy.
LazyLockrequires a relatively recent Rust (>=1.70). If your MSRV is lower or aligned to DataFusion consumers, preferonce_cell::sync::Lazyto avoid toolchain friction.If needed, apply this diff:
- static INSTANCE: std::sync::LazyLock< - std::sync::Arc<datafusion::logical_expr::ScalarUDF>, - > = std::sync::LazyLock::new(|| { - std::sync::Arc::new(datafusion::logical_expr::ScalarUDF::new_from_impl( - <$UDF>::new(), - )) - }); + static INSTANCE: once_cell::sync::Lazy< + std::sync::Arc<datafusion::logical_expr::ScalarUDF>, + > = once_cell::sync::Lazy::new(|| { + std::sync::Arc::new(datafusion::logical_expr::ScalarUDF::new_from_impl( + <$UDF>::new(), + )) + });Note: add once_cell as a dependency if not already present.
8-9: Fix doc link target.Use the same path as the code (
datafusion::logical_expr::ScalarUDF) to avoid broken intra-doc links.- #[doc = concat!("Return a [`ScalarUDF`](datafusion_expr::ScalarUDF) implementation of ", stringify!($NAME))] + #[doc = concat!("Return a [`ScalarUDF`](datafusion::logical_expr::ScalarUDF) implementation of ", stringify!($NAME))]
5-7: Consider not exporting the macro globally.If this macro is only used within this crate, avoid
#[macro_export]to prevent leaking it into downstream crates. Use path-qualified invocation instead.-#[macro_export] -macro_rules! make_udf_function { +// keep crate-local, invoke as `crate::mdl::function::macros::make_udf_function!` +macro_rules! make_udf_function {wren-core/core/src/mdl/function/aggregate/mod.rs (1)
11-51: Optionally cache the registry to avoid repeated Vec allocation.If
aggregate_functions()is called frequently, consider aLazy<Vec<Arc<AggregateUDF>>>and return a clone.Example:
use once_cell::sync::Lazy; static AGG_FUNCS: Lazy<Vec<Arc<AggregateUDF>>> = Lazy::new(|| vec![ // ... ]); pub fn aggregate_functions() -> Vec<Arc<AggregateUDF>> { AGG_FUNCS.clone() }wren-core/sqllogictest/src/test_context.rs (2)
81-89: Simplify Result handling; avoid.ok().unwrap().Use direct
unwrap()on the Result.- let ctx = apply_wren_on_ctx( + let ctx = apply_wren_on_ctx( &ctx, mdl.clone(), SessionPropertiesRef::default(), Mode::LocalRuntime, ) - .await - .ok() - .unwrap(); + .await + .unwrap();
314-321: UseSessionPropertiesRef::default()for consistency.Minor readability improvement over
Arc::new(HashMap::new()).- let ctx = apply_wren_on_ctx( + let ctx = apply_wren_on_ctx( ctx, Arc::clone(&analyzed_mdl), - Arc::new(HashMap::new()), + SessionPropertiesRef::default(), Mode::LocalRuntime, )Also applies to: 550-556
wren-core/core/src/mdl/function/mod.rs (1)
8-11: Avoid wildcard re-exports for API stabilitypub use remote_function::* exports the entire module surface and can cause churn. Prefer exporting only needed symbols to keep a stable public API.
wren-core/wren-example/examples/to-many-calculation.rs (1)
20-26: Be explicit about CSV headers to avoid defaults driftSet has_header(true) explicitly for robustness across DataFusion versions.
- ctx.register_csv( + ctx.register_csv( "orders", "sqllogictest/tests/resources/ecommerce/orders.csv", - CsvReadOptions::new(), + CsvReadOptions::new().has_header(true), ) @@ - ctx.register_csv( + ctx.register_csv( "customers", "sqllogictest/tests/resources/ecommerce/customers.csv", - CsvReadOptions::new(), + CsvReadOptions::new().has_header(true), ) @@ - ctx.register_csv( + ctx.register_csv( "order_items", "sqllogictest/tests/resources/ecommerce/order_items.csv", - CsvReadOptions::new(), + CsvReadOptions::new().has_header(true), )Also applies to: 36-41, 51-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
ibis-server/tests/routers/v3/connector/postgres/test_query.py(1 hunks)wren-core-py/src/context.rs(6 hunks)wren-core/core/src/mdl/context.rs(1 hunks)wren-core/core/src/mdl/function/aggregate/mod.rs(1 hunks)wren-core/core/src/mdl/function/macros.rs(1 hunks)wren-core/core/src/mdl/function/mod.rs(1 hunks)wren-core/core/src/mdl/function/scalar/mod.rs(1 hunks)wren-core/core/src/mdl/function/scalar/to_char.rs(1 hunks)wren-core/core/src/mdl/function/table/mod.rs(1 hunks)wren-core/core/src/mdl/function/window/mod.rs(1 hunks)wren-core/core/src/mdl/mod.rs(81 hunks)wren-core/sqllogictest/src/test_context.rs(5 hunks)wren-core/wren-example/examples/to-many-calculation.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
ibis-server/tests/routers/v3/connector/postgres/test_query.py (3)
ibis-server/tests/conftest.py (1)
client(18-23)ibis-server/tests/routers/v3/connector/postgres/test_functions.py (1)
manifest_str(35-36)ibis-server/tests/routers/v3/connector/postgres/conftest.py (1)
connection_info(54-61)
wren-core/core/src/mdl/function/mod.rs (4)
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/table/mod.rs (1)
table_functions(9-11)wren-core/core/src/mdl/function/window/mod.rs (1)
window_functions(5-19)
wren-core-py/src/context.rs (3)
wren-core/core/src/mdl/mod.rs (4)
mdl(197-232)create_wren_ctx(367-382)new(128-183)new(553-555)wren-core/core/src/mdl/context.rs (2)
apply_wren_on_ctx(46-117)new(336-366)wren-core-py/src/remote_functions.rs (2)
from(65-99)from(103-139)
wren-core/core/src/mdl/function/macros.rs (1)
wren-core/core/src/mdl/function/scalar/to_char.rs (1)
new(27-50)
wren-core/core/src/mdl/mod.rs (6)
wren-core/core/src/mdl/context.rs (3)
apply_wren_on_ctx(46-117)new(336-366)properties(93-99)wren-core-py/src/context.rs (4)
new(81-203)remote_functions(88-91)register_remote_function(282-301)default(62-70)wren-core/core/src/mdl/function/scalar/mod.rs (1)
scalar_functions(18-192)wren-core/core/src/mdl/function/aggregate/mod.rs (1)
aggregate_functions(11-51)wren-core/core/src/mdl/function/window/mod.rs (1)
window_functions(5-19)wren-core/core/src/mdl/function/table/mod.rs (1)
table_functions(9-11)
wren-core/wren-example/examples/to-many-calculation.rs (2)
wren-core/core/src/mdl/mod.rs (2)
mdl(197-232)create_wren_ctx(367-382)wren-core/core/src/mdl/context.rs (1)
apply_wren_on_ctx(46-117)
wren-core/sqllogictest/src/test_context.rs (2)
wren-core/core/src/mdl/mod.rs (2)
mdl(197-232)create_wren_ctx(367-382)wren-core/core/src/mdl/context.rs (1)
apply_wren_on_ctx(46-117)
⏰ 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 (7)
wren-core/core/src/mdl/function/table/mod.rs (1)
1-11: LGTM!Clean implementation of the table function registry. Both
generate_series()andrange()are standard DataFusion table functions.wren-core/core/src/mdl/function/window/mod.rs (1)
1-19: LGTM!Comprehensive window function registry covering all standard DataFusion window functions.
wren-core/core/src/mdl/mod.rs (1)
366-382: LGTM!Well-structured helper function that centralizes SessionContext creation with all Wren MDL function registries (scalar, aggregate, window, table). This promotes consistency and reduces duplication across the codebase.
wren-core/core/src/mdl/context.rs (1)
46-51: API rename/addition looks good.
apply_wren_on_ctxwithModethreads analyze/optimize behavior cleanly and preserves prior configuration handling.wren-core/sqllogictest/src/test_context.rs (1)
66-67: Good switch tocreate_wren_ctx.Using
create_wren_ctx(Some(config))aligns tests with the new registration flow.wren-core-py/src/context.rs (1)
35-35: API migration to create_wren_ctx/apply_wren_on_ctx looks correctImport, context creation, and both Unparse/LocalRuntime applications are wired properly; get_available_functions uses exec_ctx as expected.
Also applies to: 93-99, 172-188, 229-230
wren-core/wren-example/examples/to-many-calculation.rs (1)
79-85: Context application flow LGTMcreate_wren_ctx then apply_wren_on_ctx with Mode::LocalRuntime is correct; tables registered before apply are preserved via state cloning.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
wren-core/core/src/mdl/mod.rs (1)
3735-3775: Test will fail:to_charimplementation is incomplete.This test verifies that
to_charcalls for both date/time formatting (line 3762) and numeric formatting (line 3768) are correctly transformed into SQL. However, as noted in a previous review, the actualto_charimplementation inwren-core/core/src/mdl/function/scalar/to_char.rsstill returnsnot_impl_err!, so any attempt to execute this function will fail at runtime.The test only validates SQL transformation, not execution correctness. To make this test meaningful, you must first complete the
to_charimplementation to handle both temporal and numeric formatting before this test can pass end-to-end execution.Based on learnings
wren-core/core/src/mdl/function/scalar/to_char.rs (2)
45-50: Document numeric-format behaviour too.Docs still describe only Chrono/strftime formats even though the signature accepts integers/floats/decimals. Please add the PostgreSQL-style numeric format description and examples so users know how numerics are formatted.
72-74: Stub still blocks use.
invoke_with_argsreturnsnot_impl_err!, so everyto_char(...)call fails. Implement formatting for the supported value types (dates/times/durations + numerics) instead of returningnot_impl_err!.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wren-core/core/src/mdl/function/scalar/to_char.rs(1 hunks)wren-core/core/src/mdl/mod.rs(81 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
wren-core/core/src/mdl/mod.rs (7)
wren-core/core/src/mdl/context.rs (3)
apply_wren_on_ctx(46-117)new(336-366)properties(93-99)wren-core/core/src/mdl/function/scalar/to_char.rs (2)
new(29-52)default(23-25)wren-core-py/src/context.rs (4)
new(81-203)remote_functions(88-91)register_remote_function(282-301)default(62-70)wren-core/core/src/mdl/function/scalar/mod.rs (1)
scalar_functions(18-192)wren-core/core/src/mdl/function/aggregate/mod.rs (1)
aggregate_functions(11-51)wren-core/core/src/mdl/function/window/mod.rs (1)
window_functions(5-19)wren-core/core/src/mdl/function/table/mod.rs (1)
table_functions(9-11)
⏰ 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). (6)
- GitHub Check: check Cargo.toml formatting
- GitHub Check: cargo test (arm64)
- GitHub Check: cargo test (amd64)
- GitHub Check: clippy
- GitHub Check: ci
- GitHub Check: test
🔇 Additional comments (2)
wren-core/core/src/mdl/mod.rs (2)
366-382: LGTM! Centralized function registration.The new
create_wren_ctxfunction centralizes SessionContext creation with all default function registrations. The builder pattern is used appropriately, and the optional config parameter provides flexibility.
393-393: LGTM! Consistent API refactoring.All test functions have been consistently updated to use the new
create_wren_ctx(None)helper instead of direct SessionContext instantiation, and calls have been updated to use the renamedapply_wren_on_ctxAPI. The refactoring is thorough and consistent.Also applies to: 480-480, 647-647, 680-680, 712-712, 723-723, 737-737, 757-757, 778-778, 837-837, 876-876, 890-890, 903-903, 917-917, 946-946, 962-962, 980-980, 1005-1005, 1016-1016, 1028-1028, 1047-1047, 1078-1078, 1093-1093, 1113-1113, 1128-1128, 1147-1147, 1169-1169, 1211-1211, 1252-1252, 1283-1283, 1309-1309, 1343-1343, 1362-1362, 1391-1391, 1409-1409, 1460-1460, 1487-1487, 1502-1502, 1516-1516, 1531-1531, 1547-1547, 1579-1579, 1678-1678, 1697-1697, 1728-1728, 1744-1744, 1782-1782, 1817-1817, 1852-1852, 1914-1914, 1965-1965, 1989-1989, 2201-2201, 2563-2563, 2626-2626, 2666-2666, 2714-2714, 2811-2811, 2884-2884, 2926-2926, 3034-3034, 3186-3186, 3224-3224, 3254-3254, 3315-3315, 3374-3374, 3458-3458, 3495-3495, 3525-3525, 3568-3568, 3631-3631, 3665-3665, 3684-3684, 3737-3737
goldmedal
force-pushed
the
feat/to-char-string
branch
from
64a3ecf to
9bdc268
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
wren-core/core/src/mdl/function/scalar/to_char.rs (3)
40-41: Accept any Decimal precision/scale, not just (38,10).Hard-coding Decimal128/256 to (38,10) rejects valid types like DECIMAL(12,2). Use a Decimal class signature so any decimal coerces with a Utf8 format string.
- TypeSignature::Exact(vec![DataType::Decimal128(38, 10), DataType::Utf8]), - TypeSignature::Exact(vec![DataType::Decimal256(38, 10), DataType::Utf8]), + TypeSignature::Coercible(vec![ + Coercion::new_exact(TypeSignatureClass::Decimal), + Coercion::new_exact(TypeSignatureClass::Native(logical_string())) + ]),If TypeSignatureClass::Decimal isn’t available in your version, reply and I’ll adapt this to the project’s decimal matching pattern.
45-50: Docs: mention numeric formatting and examples.Signature supports integers/floats/decimals but docs only cover Chrono formats. Clarify that:
- Chrono patterns apply to date/time/timestamp/duration
- PostgreSQL-style patterns apply to numerics (9/0 placeholders, decimal point, rounding, optional FM modifier)
Add 2–3 examples.- "Returns a string representation of a date, time, timestamp or duration based on a [Chrono format](https://docs.rs/chrono/latest/chrono/format/strftime/index.html).", - "to_char(expression, format)") - .with_argument("expression", "The date, time, timestamp or duration to format.") - .with_argument("format", "The [Chrono format](https://docs.rs/chrono/latest/chrono/format/strftime/index.html) string to use for formatting.") + "Format values as text. For date/time types, use [Chrono strftime](https://docs.rs/chrono/latest/chrono/format/strftime/index.html). For numeric types, use PostgreSQL-style patterns (e.g., 9/0 for digit placeholders, '.' for decimal point, optional FM).", + "to_char(expression, format)") + .with_argument("expression", "Value to format (date/time/timestamp/duration or integer/float/decimal).") + .with_argument("format", "Chrono format for temporal values; PostgreSQL-style pattern for numerics (e.g., '9999', '0000.00', 'FM999.99').") + .with_example("to_char(1234, '9999') -> '1234'") + .with_example("to_char(1234.567, '0000.00') -> '1234.57'") + .with_example(\"to_char(timestamp '2024-01-01 23:59:59', '%Y-%m-%d %H:%M:%S') -> '2024-01-01 23:59:59'\")
72-74: Implement invoke_with_args (blocking).Function currently returns not_impl_err; all new tests will fail. Implement formatting for:
- date/time/timestamp/duration using Chrono patterns
- integer/float/decimal using PostgreSQL-style numeric patterns ('9', '0', '.', grouping, rounding for scale)
Return Utf8. Handle NULLs, arity/type checks, and errors without panics. I can provide a minimal numeric formatter to pass current tests ('9999', '0000.00') if helpful.ibis-server/tests/routers/v3/connector/postgres/test_query.py (1)
1284-1301: Tests will fail: to_char UDF still returns not_impl_err.The new assertions depend on a working to_char, but wren-core/core/src/mdl/function/scalar/to_char.rs::invoke_with_args still returns not_impl_err. Either implement it or skip these tests until it’s ready.
🧹 Nitpick comments (1)
wren-core/core/src/mdl/function/macros.rs (1)
8-8: Fix doc link to match the actual type path.The code uses datafusion::logical_expr::ScalarUDF; the doc link points to datafusion_expr::ScalarUDF, which may break rustdoc unless that crate is a dependency. Point the link at the used path.
- #[doc = concat!("Return a [`ScalarUDF`](datafusion_expr::ScalarUDF) implementation of ", stringify!($NAME))] + #[doc = concat!("Return a [`ScalarUDF`](datafusion::logical_expr::ScalarUDF) implementation of ", stringify!($NAME))]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ibis-server/tests/routers/v3/connector/postgres/test_query.py(1 hunks)wren-core/core/src/mdl/function/macros.rs(1 hunks)wren-core/core/src/mdl/function/mod.rs(1 hunks)wren-core/core/src/mdl/function/scalar/mod.rs(1 hunks)wren-core/core/src/mdl/function/scalar/to_char.rs(1 hunks)wren-core/core/src/mdl/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- wren-core/core/src/mdl/mod.rs
- wren-core/core/src/mdl/function/scalar/mod.rs
🧰 Additional context used
🧬 Code graph analysis (2)
ibis-server/tests/routers/v3/connector/postgres/test_query.py (3)
ibis-server/tests/conftest.py (1)
client(18-23)ibis-server/tests/routers/v3/connector/postgres/test_functions.py (1)
manifest_str(35-36)ibis-server/tests/routers/v3/connector/postgres/conftest.py (1)
connection_info(54-61)
wren-core/core/src/mdl/function/macros.rs (1)
wren-core/core/src/mdl/function/scalar/to_char.rs (1)
new(29-52)
🔇 Additional comments (2)
wren-core/core/src/mdl/function/mod.rs (1)
2-2: LGTM: macros module inclusion is fine.ibis-server/tests/routers/v3/connector/postgres/test_query.py (1)
1296-1297: Remove leading spaces from test assertions; PostgreSQL does not add them for these inputs.The web search confirms that PostgreSQL's to_char(1234, '9999') returns '1234' and to_char(1234.567, '0000.00') returns '1234.57' without leading spaces. While by default output is right-aligned with leading spaces, this padding applies only when the value requires right-alignment within the template width. Since both test values fit exactly within their format templates, no leading spaces are added.
Update lines 1296–1297:
- assert result["data"][0][0] == " 1234" - assert result["data"][0][1] == " 1234.57" + assert result["data"][0][0] == "1234" + assert result["data"][0][1] == "1234.57"Likely an incorrect or invalid review comment.
2 participants
follow up #1346
Summary by CodeRabbit
New Features
to_charSQL function for formatting date/time and numeric values with Chrono-format patterns. Supports multiple data types including dates, timestamps, durations, and decimal numbers.Tests
to_charfunctionality with PostgreSQL and numeric formatting scenarios.