Add PostgreSQL and SQLite SQL standard library#4068
Conversation
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Binary size checks failed❌ 6 violations · ✅ 1 passed
Details & how to fixViolations:
Add/update baselines:
[artifacts.baml-cli]
file_bytes = 22172080
stripped_bytes = 22172144
gzip_bytes = 10746252
[artifacts.packed-program]
file_bytes = 15304098
gzip_bytes = 7218176
[artifacts.baml-cli]
file_bytes = 23891968
stripped_bytes = 23891968
gzip_bytes = 11008241
[artifacts.packed-program]
file_bytes = 16286681
gzip_bytes = 7336817
[artifacts.baml-cli]
file_bytes = 28321552
stripped_bytes = 28321544
gzip_bytes = 12226163
[artifacts.packed-program]
file_bytes = 19319368
gzip_bytes = 8141914Generated by |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
e27f870 to
51fc2a9
Compare
📝 WalkthroughWalkthroughThis PR adds a BAML SQL standard library with PostgreSQL and SQLite support, typed statements and bindings, transactions, decoding, structured errors, native sysops, compiler wiring, integration tests, and PostgreSQL-backed CI coverage. ChangesSQL runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant BamlSqlApi
participant BamlNamespaceSql
participant NativeSysOps
participant sqlx
BamlSqlApi->>BamlNamespaceSql: create statement and connect database
BamlNamespaceSql->>NativeSysOps: dispatch SQL sysop
NativeSysOps->>sqlx: execute query or transaction operation
sqlx-->>NativeSysOps: rows, command result, or SQL error
NativeSysOps-->>BamlSqlApi: decoded value or SqlError
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
baml_language/crates/sys_types/src/lib.rs (1)
186-236: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared helper for
BamlThrown/HostThrowndisplay formatting.The new
BamlThrownarm duplicates theHostThrownarm's instance/message-extraction logic verbatim, differing only in the "thrown" vs "host thrown" prefix.♻️ Proposed refactor to de-duplicate the formatting logic
+fn format_thrown_instance( + f: &mut std::fmt::Formatter<'_>, + prefix: &str, + boxed: &BexExternalValue, +) -> std::fmt::Result { + match boxed { + BexExternalValue::Instance { + class_name, fields, .. + } => { + let message = fields.get("message").and_then(|v| match v { + BexExternalValue::String(s) => Some(s.as_str()), + _ => None, + }); + match message { + Some(m) => write!(f, "{prefix} {class_name}: {m}"), + None => write!(f, "{prefix} {class_name}"), + } + } + other => write!(f, "{prefix} {other:?}"), + } +} + fn display_summary(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Vm(e) => std::fmt::Display::fmt(e, f), - Self::BamlThrown(boxed) => match boxed.as_ref() { - BexExternalValue::Instance { - class_name, fields, .. - } => { - let message = fields.get("message").and_then(|v| match v { - BexExternalValue::String(s) => Some(s.as_str()), - _ => None, - }); - match message { - Some(m) => write!(f, "thrown {class_name}: {m}"), - None => write!(f, "thrown {class_name}"), - } - } - other => write!(f, "thrown {other:?}"), - }, - Self::HostThrown(boxed) => match boxed.as_ref() { - BexExternalValue::Instance { - class_name, fields, .. - } => { - let message = fields.get("message").and_then(|v| match v { - BexExternalValue::String(s) => Some(s.as_str()), - _ => None, - }); - match message { - Some(m) => write!(f, "host thrown {class_name}: {m}"), - None => write!(f, "host thrown {class_name}"), - } - } - other => write!(f, "host thrown {other:?}"), - }, + Self::BamlThrown(boxed) => format_thrown_instance(f, "thrown", boxed), + Self::HostThrown(boxed) => format_thrown_instance(f, "host thrown", boxed), } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/sys_types/src/lib.rs` around lines 186 - 236, Extract the duplicated instance/message formatting from OpErrorPayload::display_summary into a shared helper that accepts the throw-label prefix and BexExternalValue reference. Update both BamlThrown and HostThrown arms to call the helper with their respective “thrown” and “host thrown” labels, preserving the existing output for instance messages and non-instance values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/crates/baml_builtins2_codegen/src/codegen_io.rs`:
- Around line 726-736: Make namespace identifier conversion injective by
escaping underscores before converting dots, then apply the same encoding
consistently in ns_module_ident, namespace_pascal, and namespace_symbol so
distinct namespaces cannot collide in generated modules or dispatch methods. Add
a regression test covering namespaces such as sql.sqlite and sql_sqlite and
verify their generated identifiers remain distinct.
In `@baml_language/crates/bex_vm/src/package_baml/sql.rs`:
- Around line 58-63: Update the unsupported array element-type error in the
Object::Array handling of snapshot_bind to use backend-neutral wording, removing
the hardcoded “PostgreSQL” reference while preserving the existing error
behavior.
In `@baml_language/crates/sys_native/src/sql.rs`:
- Around line 1973-2074: Update execute_transaction and query_transaction to use
the existing discard-on-drop transaction connection handling used by
begin/finalize. Ensure cancellation during any awaited describe, execute, or
row-fetch operation prevents the live connection from being reused; preserve
normal connection reuse after successful completion.
---
Nitpick comments:
In `@baml_language/crates/sys_types/src/lib.rs`:
- Around line 186-236: Extract the duplicated instance/message formatting from
OpErrorPayload::display_summary into a shared helper that accepts the
throw-label prefix and BexExternalValue reference. Update both BamlThrown and
HostThrown arms to call the helper with their respective “thrown” and “host
thrown” labels, preserving the existing output for instance messages and
non-instance values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 518bc6ac-db3d-4ced-8131-732478ca615e
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snap
📒 Files selected for processing (24)
.github/workflows/cargo-tests.reusable.yamlbaml_language/Cargo.tomlbaml_language/crates/baml_builtins2/Cargo.tomlbaml_language/crates/baml_builtins2/baml_std/baml/ns_sql/ns_postgres/postgres.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_sql/ns_sqlite/sqlite.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_sql/sql.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_builtins2/src/sql.rsbaml_language/crates/baml_builtins2_codegen/src/codegen_io.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_tests/Cargo.tomlbaml_language/crates/baml_tests/tests/sql.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_external_types/src/bex_external_value.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/sql.rsbaml_language/crates/sys_native/Cargo.tomlbaml_language/crates/sys_native/src/host_impls.rsbaml_language/crates/sys_native/src/lib.rsbaml_language/crates/sys_native/src/sql.rsbaml_language/crates/sys_ops/src/lib.rsbaml_language/crates/sys_types/src/lib.rsbaml_language/crates/sys_wasm/src/host_value.rs
5cee29b to
2d20e88
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler2_hir/src/builder.rs (1)
2180-2192: 📐 Maintainability & Code Quality | 🔵 TrivialRun
cargo test --libfrombaml_language/. The repo root doesn’t contain a Cargo manifest, so the test command needs to target the Rust workspace directory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_compiler2_hir/src/builder.rs` around lines 2180 - 2192, Run the Rust library tests from the baml_language workspace directory using cargo test --lib, rather than running the command from the repository root where no Cargo manifest exists.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@baml_language/crates/baml_compiler2_hir/src/builder.rs`:
- Around line 2180-2192: Run the Rust library tests from the baml_language
workspace directory using cargo test --lib, rather than running the command from
the repository root where no Cargo manifest exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50921059-984e-4f5d-ad29-b499023df07a
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snap
📒 Files selected for processing (24)
.github/workflows/cargo-tests.reusable.yamlbaml_language/Cargo.tomlbaml_language/crates/baml_builtins2/Cargo.tomlbaml_language/crates/baml_builtins2/baml_std/baml/ns_sql/ns_postgres/postgres.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_sql/ns_sqlite/sqlite.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_sql/sql.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_builtins2/src/sql.rsbaml_language/crates/baml_builtins2_codegen/src/codegen_io.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_tests/Cargo.tomlbaml_language/crates/baml_tests/tests/sql.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_external_types/src/bex_external_value.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/sql.rsbaml_language/crates/sys_native/Cargo.tomlbaml_language/crates/sys_native/src/host_impls.rsbaml_language/crates/sys_native/src/lib.rsbaml_language/crates/sys_native/src/sql.rsbaml_language/crates/sys_ops/src/lib.rsbaml_language/crates/sys_types/src/lib.rsbaml_language/crates/sys_wasm/src/host_value.rs
🚧 Files skipped from review as they are similar to previous changes (22)
- baml_language/crates/baml_builtins2/Cargo.toml
- baml_language/crates/sys_native/src/lib.rs
- baml_language/crates/baml_builtins2_codegen/src/extract.rs
- baml_language/crates/baml_tests/Cargo.toml
- baml_language/crates/bex_external_types/src/bex_external_value.rs
- baml_language/crates/sys_native/src/host_impls.rs
- baml_language/crates/sys_wasm/src/host_value.rs
- baml_language/Cargo.toml
- baml_language/crates/baml_builtins2/src/lib.rs
- baml_language/crates/baml_builtins2/src/sql.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_sql/ns_postgres/postgres.baml
- baml_language/crates/bex_vm/src/package_baml/mod.rs
- baml_language/crates/bex_engine/src/lib.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_sql/ns_sqlite/sqlite.baml
- baml_language/crates/sys_ops/src/lib.rs
- baml_language/crates/sys_types/src/lib.rs
- baml_language/crates/sys_native/Cargo.toml
- baml_language/crates/bex_vm/src/package_baml/sql.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_sql/sql.baml
- .github/workflows/cargo-tests.reusable.yaml
- baml_language/crates/baml_builtins2_codegen/src/codegen_io.rs
- baml_language/crates/baml_tests/tests/sql.rs
Summary
baml.sqlstandard-library API with tagged, parameterized statementserrors
timeouts, and extended error codes
Why
Implements BEP-047, providing a safe native SQL API where interpolated values are always bound
parameters and query results decode using runtime BAML types.
Validation
cargo fmt --all -- --check-D warningsacross all changed crates and targetsSummary by CodeRabbit