feat: add SurrealDB support (PSL, query compiler, schema engine)#5798
Open
Mikefluff wants to merge 11 commits intoprisma:mainfrom
Open
feat: add SurrealDB support (PSL, query compiler, schema engine)#5798Mikefluff wants to merge 11 commits intoprisma:mainfrom
Mikefluff wants to merge 11 commits intoprisma:mainfrom
Conversation
Full SurrealDB integration across the Rust codebase: **PSL Parser:** - `Flavour::SurrealDb` variant in datamodel connector - `SurrealDbDatamodelConnector` implementing `Connector` trait (no native types, no FK constraints, `relationMode = "prisma"`) - Feature flag `surrealdb` in psl-core and psl **Query Compiler (quaint):** - `SqlFamily::SurrealDb` variant with all match arms - `AdapterProvider::SurrealDb` in external connector - SurrealQL visitor (~540 lines) with key differences from SQL: - `$1`, `$2` placeholders (PostgreSQL-style) - `DELETE table` without FROM keyword - `LIMIT ... START` instead of `LIMIT ... OFFSET` - `RETURN` instead of `RETURNING` - `string::concat()` instead of `||` - Query compiler dispatch for SurrealDB - Wasm build configuration (build.sh, Makefile) **Schema Engine:** - `SurrealDbDialect` implementing `SqlDialect` - `SurrealDbConnector` implementing `SqlConnector` (external adapter only) - `SurrealDbRenderer` — DDL generation: `DEFINE TABLE SCHEMAFULL`, `DEFINE FIELD ON TABLE TYPE`, `DEFINE INDEX ON TABLE FIELDS UNIQUE`, `REMOVE TABLE/FIELD/INDEX` - `SurrealDbSchemaCalculatorFlavour`, `SurrealDbSchemaDifferFlavour`, `SurrealDbDestructiveChangeCheckerFlavour` - `SurrealDbIntrospectionFlavour` - Migrations table using SurrealQL (`DEFINE TABLE _prisma_migrations`) **Verification:** - `cargo check` passes for all 6 affected crates - 7/7 quaint SurrealQL visitor tests pass Companion PR: prisma/prisma#29367 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds SurrealDB support across the codebase: new Cargo features, a datamodel connector and flavour, quaint SQL family and visitor, query-compiler/WASM wiring and size reporting, and extensive schema-engine additions (dialect, connector, renderer, differ, calculator, destructive checks, and introspection wiring). Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
query-compiler/query-compiler-wasm/src/params.rs (1)
48-61:⚠️ Potential issue | 🟠 MajorAdd explicit
#[serde(rename = "surrealdb")]to theSurrealDbenum variant.With
#[serde(rename_all = "camelCase")]on the enum, the variant will deserialize from"surrealDb", but the codebase consistently uses lowercase"surrealdb"everywhere else:db_system_name(),provider_name(), andFromStrimplementations all expect lowercase. This will cause deserialization failures.Note: SurrealDB support appears incomplete in the wasm layer—it's not included in the
CONNECTOR_REGISTRYor the TypeScriptQueryCompilerParamstype definition (provider: 'postgres' | 'mysql' | 'sqlite' | 'sqlserver'), so broader integration work may be needed.Suggested fix
#[cfg(feature = "surrealdb")] + #[serde(rename = "surrealdb")] SurrealDb,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@query-compiler/query-compiler-wasm/src/params.rs` around lines 48 - 61, The SurrealDb enum variant currently derives camelCase renaming which makes it deserialize from "surrealDb" but the codebase expects "surrealdb"; update the AdapterProvider::SurrealDb variant by adding an explicit serde rename attribute to serialize/deserialize as "surrealdb" (i.e., add #[serde(rename = "surrealdb")] above the SurrealDb variant) so deserialization aligns with db_system_name(), provider_name(), and FromStr expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 69-71: The measure-qc-wasm-% Makefile target is missing
"surrealdb" in its provider loop while build-qc-gz-% includes it; update the
provider list in the measure-qc-wasm-% target so it matches build-qc-gz-% (add
surrealdb to the loop that iterates providers in measure-qc-wasm-%) to ensure
SurrealDB WASM artifacts are measured and reported.
In `@quaint/src/connector/connection_info.rs`:
- Around line 417-418: Add a feature-gated helper on the SqlFamily enum to
mirror the existing helpers: implement pub fn is_surrealdb(&self) -> bool with
#[cfg(feature = "surrealdb")] returning matches!(self, SqlFamily::SurrealDb) and
a #[cfg(not(feature = "surrealdb"))] stub returning false; include the same doc
comment style ("True, if family is SurrealDB.") and place the methods alongside
the other is_* helpers so callers can rely on consistent API regardless of
feature flags.
- Around line 512-513: The hardcoded PostgreSQL-derived value for SurrealDB in
the SqlFamily match (SqlFamily::SurrealDb => 32766) should be removed or made
explicit/justified; replace the 32766 with a conservative, documented fallback
(for example 1024) or return a sentinel/None if your API supports Option (so
callers know the limit is transport-dependent), and add a brief inline comment
explaining that SurrealDB has no documented parameter limit and that the chosen
value is a conservative transport/body-size-based fallback; update the match arm
for SqlFamily::SurrealDb accordingly and ensure any callers that rely on this
value handle the new fallback/Option behavior.
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 303-327: The SurrealDB JSON handlers are incorrect: update
visit_json_extract to output SurrealDB dot-notation (use e.g. "<column>.<path>"
semantics) instead of PostgreSQL "->"/"->>", support string and array path
segments without panicking (split the JsonPath and join with '.' or implement
proper array access if SurrealDB supports it), implement
visit_json_array_contains to emit a valid SurrealDB expression for array
containment, and fix visit_json_type_equals to map the incoming json_type to the
correct SurrealDB type check syntax (do not always write "type::is::string" and
do not perform string comparisons against predicates); alternatively, if you
choose to remove JSON support, remove the JsonFiltering and
JsonFilteringJsonPath capability flags and prevent these code paths from being
compiled/exposed. Ensure you modify the functions visit_json_extract,
visit_json_array_contains, and visit_json_type_equals accordingly.
In `@query-compiler/query-compiler-wasm/Cargo.toml`:
- Line 41: AdapterProvider::SurrealDb currently relies on #[serde(rename_all =
"camelCase")] which maps it to "surrealDb" but the provider/feature naming uses
"surrealdb"; update the enum variant SurrealDb in params.rs to include an
explicit serde rename by adding #[serde(rename = "surrealdb")] above the
SurrealDb variant so deserialization recognizes the lowercase provider name.
In `@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb.rs`:
- Around line 129-131: The acquire_lock implementation currently returns Ok(())
without any synchronization, which can cause race conditions during concurrent
migrations; either (A) implement an advisory lock using SurrealDB's facilities
in the acquire_lock method (and corresponding release) by performing a
lightweight transactional/row-lease or CAS-style operation via the SurrealDB
client (use the SurrealDB session/transaction API inside function acquire_lock
and ensure a matching release/unlock path), or (B) if you can't implement
locking now, add an explicit docstring/TODO comment on the acquire_lock function
and update the connector README to state this limitation and warn that
migrations are not safe for concurrent processes; locate the method named
acquire_lock in surrealdb.rs and apply one of these fixes so callers are aware
or protected.
- Around line 218-227: The function sql_schema_from_migration_history in
surrealdb.rs currently returns SqlSchema::default(), which disables drift
detection; replace this stub with a real implementation that reconstructs the
expected SqlSchema from the given Migrations, Namespaces, SchemaFilter and
UsingExternalShadowDb (i.e., replay/parse the migration history or query a
proper shadow DB) so the engine can compute diffs, or if a full implementation
is not possible now, change the function to return a clear unimplemented
ConnectorResult error (rather than a silent default) and add a TODO comment that
references a new tracking issue created for "full shadow DB /
migration-history-based schema for SurrealDB" so future work is discoverable;
touch function sql_schema_from_migration_history and include the tracking issue
ID in the comment.
- Around line 157-170: In create_migrations_table change the SurrealQL field
clauses for _prisma_migrations so that started_at uses DEFAULT time::now()
instead of VALUE (to set timestamp only on creation) and change
applied_steps_count to not use VALUE 0 (either remove the VALUE clause and
initialize from application logic or use DEFAULT 0 if you want a one-time
creation default); update the DEFINE FIELD lines for started_at and
applied_steps_count accordingly.
- Around line 145-151: The table_names stub returns an empty Vec which bypasses
safety checks; implement real enumeration in the table_names method by issuing
SurrealDB's "INFO FOR DB" (or equivalent client call) to list tables for the
current namespace/database, parse the response into Vec<String>, and return
Ok(names) or map errors into ConnectorResult; ensure you honor the provided
_namespaces/_filters parameters or explicitly ignore them with a clear comment.
If you cannot implement now, replace the stub with a clear TODO and a doc
comment referencing a tracking issue and return an Err with a descriptive
ConnectorResult error instead of silently returning an empty list to avoid
masking existing tables.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rs`:
- Around line 137-139: The convert_error function currently calls
quaint_error_to_connector_error(err, None) losing connection context; update
convert_error to retrieve and pass the relevant connection_info (from an
available State or Connection object) into quaint_error_to_connector_error
instead of None—e.g., change convert_error to accept a &State or &Connection (or
access it where convert_error is called), extract the connection info and
forward it to quaint_error_to_connector_error so errors retain connection
context.
- Around line 111-115: The current wasm::introspect function (introspect(state:
&mut State) -> ConnectorResult<SqlSchema>) returns SqlSchema::default(),
preventing schema diffing; replace this stub with either a real minimal
introspection that executes SurrealDB's "INFO FOR DB" and "INFO FOR TABLE"
against the provided State and parses table/field metadata into a SqlSchema, or
return an explicit ConnectorResult::Err indicating introspection is
unimplemented. Locate the wasm::introspect implementation in mod.rs,
reuse/parity-check parsing logic from the parent surrealdb.rs introspection
helpers (or the same parsing functions) to build a SqlSchema from the INFO
responses, and ensure errors propagate as ConnectorResult failures rather than
silently returning an empty schema.
- Around line 39-43: The query function currently calls
SurrealDb::build(query).unwrap(), which can panic; change it to handle the
Result returned by quaint::visitor::SurrealDb::build and propagate failures into
the ConnectorResult instead of unwrapping. In the pub async fn query(&self,
query: quaint::ast::Query<'_>) -> ConnectorResult<quaint::prelude::ResultSet>
function, replace the unwrap with proper error handling (use the ? operator or
map_err to convert the visitor error into the ConnectorResult error type) before
calling self.query_raw(&sql, ¶ms).await so malformed ASTs return a
ConnectorResult::Err rather than panicking.
- Around line 71-82: The reset() implementation is a no-op after calling
self.adapter.query_raw("INFO FOR DB", &[]) — either implement dropping tables or
fail fast; update the reset(&self, _params: &Params) -> ConnectorResult<()> in
mod.rs to parse the query_raw result, extract table names (from the returned
rows), and for each table call self.adapter.query_raw("DROP TABLE <table>",
&[]).await.map_err(convert_error) (or batch a DROP for each name) and propagate
any errors via convert_error, ensuring awaits and mapping are correct; if you
prefer not to support it yet, return an explicit Err indicating reset is
unsupported instead of Ok(()) so callers see the operation isn't implemented.
- Around line 100-103: The drop_database function currently no-ops and returns
Ok(()), leaving the database intact; update the drop_database(State)
implementation to either execute SurrealDB's "REMOVE DATABASE" through the
existing client/accessor in this module (use the same client access pattern as
other functions in this file to run the SQL command and map errors into
ConnectorResult) or, if removal is unsupported in the WASM flavour, return an
explicit Err with a clear message; reference the drop_database function, State
parameter, and ConnectorResult type when making the change so callers see the
implemented behavior or a deterministic error.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/destructive_change_checker.rs`:
- Around line 101-115: The two stub functions count_rows_in_table and
count_values_in_column currently always return 0 which falsely marks tables as
empty; replace these stubs with real SurrealDB counts by using the provided
SqlConnector to run an appropriate COUNT query and return the numeric result as
ConnectorResult<i64>, or if implementing queries is not possible yet, return a
conservative failure (Err with a descriptive ConnectorError) or a non-zero
conservative fallback so destructive checks treat tables as populated; ensure
the changes are made inside the count_rows_in_table and count_values_in_column
implementations and propagate any query errors rather than swallowing them.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs`:
- Around line 259-260: ColumnTypeFamily::BigInt is currently rendered as "int"
which can truncate values beyond 64-bit; update the mapping in renderer.rs so
ColumnTypeFamily::BigInt renders as "string" (or, if you intentionally keep
"int", add a clear comment/logging documenting the 64-bit SurrealDB limit) to
avoid silent precision loss — change the match arm for ColumnTypeFamily::BigInt
in the renderer function that produces SQL types accordingly and add a brief
comment referencing the 64-bit limitation.
- Around line 230-237: render_rename_table currently returns a comment string
which silently "succeeds"; change it to return an explicit error instead of a
comment by updating fn render_rename_table(...) -> String to return a
Result<String, RenderError> (or the crate's existing error type), and return
Err(RenderError::new("SurrealDB does not support RENAME TABLE; manual migration
required or use copy-and-drop migration")). Update all callers of
render_rename_table to propagate or handle the Result (e.g., using ?), and add a
unit test asserting an error is produced for rename attempts; alternatively, if
you prefer automated migration, implement copy-and-drop SQL inside
render_rename_table (create new table AS SELECT * FROM old, recreate
indexes/constraints, then DROP old) and return that SQL string instead, but be
explicit about handling indexes/foreign-keys and document in the function
comment.
- Line 286: The match arm using unreachable!() for
DefaultKind::DbGenerated(None) | DefaultKind::Sequence(_) |
DefaultKind::UniqueRowid in renderer.rs can panic at runtime when a model uses
`@default`(autoincrement()) (UniqueRowid); change this arm to handle UniqueRowid
safely by returning a sensible placeholder or a clear error instead of
panicking—e.g., map DefaultKind::UniqueRowid to a SurrealDB-compatible default
string or return a Result::Err with a descriptive message from the function that
renders defaults (update the renderer function signature or its callers to
propagate the error if necessary); ensure the same safe handling is applied for
DbGenerated(None) and Sequence(_) so no runtime panic can occur.
- Around line 78-84: The AddColumn path currently emits the default as a SQL
comment using the statements.push(format!(... "-- Default value for
{field_name}: {default_val}" ...)), so change it to append a proper VALUE clause
to the column definition instead: when column.default() is Some(default) call
render_default(default.inner()) and include it as " VALUE {rendered_default}" in
the same statement that defines the column (mirroring how render_create_table_as
does it). Update the code around AddColumn handling (look for AddColumn,
column.default(), render_default, and the existing
statements.push(format!(...))) to remove the comment and concatenate the " VALUE
<rendered_default>" fragment into the column definition string.
- Around line 264-268: Update the SurrealDB type mapping in the renderer so
UUIDs use SurrealDB's native type: locate the match arm in the SurrealDB
renderer that maps ColumnTypeFamily (the match handling ColumnTypeFamily::Enum,
::Uuid, ::Udt, ::Unsupported) and change the ColumnTypeFamily::Uuid arm from
returning "string" to returning "uuid" (so the renderer emits DEFINE FIELD ...
TYPE uuid for UUID columns).
- Around line 70-76: DropAndRecreateColumn is using tables.next.name() when
emitting the "DEFINE FIELD ... ON TABLE {table_name}" statement, causing a
mismatch with AddColumn which uses tables.previous.name(); change the table
reference in DropAndRecreateColumn to use tables.previous.name() so both
operations target the same table within the same AlterTable. Locate the format
call that builds "DEFINE FIELD {field_name} ON TABLE {table_name} TYPE
{field_type}{nullable}" inside the DropAndRecreateColumn handling in renderer.rs
and replace tables.next.name() with tables.previous.name().
In `@schema-engine/connectors/sql-schema-connector/src/lib.rs`:
- Around line 348-355: The SurrealDB external-adapter constructor is never used
because new_from_external does not route SurrealDB adapters to
SqlSchemaConnector::new_surrealdb_external; update new_from_external to detect
the SurrealDB external adapter case and call
SqlSchemaConnector::new_surrealdb_external (which constructs
flavour::SurrealDbConnector::new_external) instead of falling through to the
Postgres/SQLite branches and panicking, ensuring the external SurrealDB path is
reachable from the SurrealDB entrypoint in
schema-engine/core::new_from_external.
---
Outside diff comments:
In `@query-compiler/query-compiler-wasm/src/params.rs`:
- Around line 48-61: The SurrealDb enum variant currently derives camelCase
renaming which makes it deserialize from "surrealDb" but the codebase expects
"surrealdb"; update the AdapterProvider::SurrealDb variant by adding an explicit
serde rename attribute to serialize/deserialize as "surrealdb" (i.e., add
#[serde(rename = "surrealdb")] above the SurrealDb variant) so deserialization
aligns with db_system_name(), provider_name(), and FromStr expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50e08414-566e-465f-82a1-98c53a393e59
📒 Files selected for processing (35)
Makefilepsl/psl-core/Cargo.tomlpsl/psl-core/src/builtin_connectors/mod.rspsl/psl-core/src/builtin_connectors/surrealdb_datamodel_connector.rspsl/psl-core/src/datamodel_connector.rspsl/psl/Cargo.tomlquaint/Cargo.tomlquaint/src/connector/connection_info.rsquaint/src/connector/external.rsquaint/src/lib.rsquaint/src/visitor.rsquaint/src/visitor/surrealdb.rsquery-compiler/query-compiler-wasm/Cargo.tomlquery-compiler/query-compiler-wasm/build.shquery-compiler/query-compiler-wasm/src/params.rsquery-compiler/query-compiler/Cargo.tomlquery-compiler/query-compiler/src/lib.rsschema-engine/commands/Cargo.tomlschema-engine/commands/src/lib.rsschema-engine/connectors/schema-connector/Cargo.tomlschema-engine/connectors/sql-schema-connector/Cargo.tomlschema-engine/connectors/sql-schema-connector/src/flavour.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/destructive_change_checker.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/schema_calculator.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/schema_differ.rsschema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context.rsschema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour.rsschema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour/surrealdb.rsschema-engine/connectors/sql-schema-connector/src/lib.rsschema-engine/core/src/lib.rsschema-engine/sql-schema-describer/Cargo.toml
Comment on lines
+512
to
+513
| #[cfg(feature = "surrealdb")] | ||
| SqlFamily::SurrealDb => 32766, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
SurrealDB maximum query parameters bind values limit
💡 Result:
SurrealDB does not document a hard limit on the number of query parameters or bind values. Official documentation on parameters (SurrealQL parameters page) describes usage but specifies no maximum count. Environment variables include a "Limits config" section, but no specific parameter or bind limit is detailed. Related limits exist in storage (e.g., SurrealKV key size, increased in recent fixes for vectors) and HTTP (e.g., Python SDK URL query length ~65KB, request body 16KiB configurable via future args). Client-side issues like WebSocket max size (default 1MB, configurable) affect large binds indirectly. For production, practical limits depend on transport (HTTP/WS body size, memory) rather than a fixed parameter count; use SDK bind methods to avoid injection and size issues.
Citations:
- 1: https://surrealdb.com/docs/surrealql/parameters
- 2: https://surrealdb.com/docs/surrealdb/cli/env
- 3: Bug: SurrealHTTP URL query length surrealdb/surrealdb.py#87
- 4: Feature: add dynamic args config for the http request body size and optimize the surrealdb client logic surrealdb/surrealdb#2995
- 5: Bug: Parameter Max Size in Websocket does not work surrealdb/surrealdb.py#111
- 6: Bug: Can't insert vectors above a certain size (dimension) - crashes DB - surrealkv issue surrealdb/surrealdb#4755
SurrealDB has no documented maximum parameter limit; reconsider the hardcoded value.
SurrealDB does not document a hard limit on query parameters or bind values. The current value of 32766 is borrowed from PostgreSQL but may not be appropriate for SurrealDB, which relies on transport-level constraints (HTTP/WebSocket body size, memory) rather than a fixed parameter count. Either use a more appropriate value for SurrealDB or document why this limit was chosen.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@quaint/src/connector/connection_info.rs` around lines 512 - 513, The
hardcoded PostgreSQL-derived value for SurrealDB in the SqlFamily match
(SqlFamily::SurrealDb => 32766) should be removed or made explicit/justified;
replace the 32766 with a conservative, documented fallback (for example 1024) or
return a sentinel/None if your API supports Option (so callers know the limit is
transport-dependent), and add a brief inline comment explaining that SurrealDB
has no documented parameter limit and that the chosen value is a conservative
transport/body-size-based fallback; update the match arm for
SqlFamily::SurrealDb accordingly and ensure any callers that rely on this value
handle the new fallback/Option behavior.
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Show resolved
Hide resolved
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Outdated
Show resolved
Hide resolved
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Show resolved
Hide resolved
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Outdated
Show resolved
Hide resolved
Critical fixes: - Add #[serde(rename = "surrealdb")] to AdapterProvider::SurrealDb (camelCase would produce "surrealDb" but codebase expects "surrealdb") - Fix JSON operations: use SurrealDB dot-notation instead of PostgreSQL ->/->> operators, implement CONTAINS for array containment, use type::is_* functions for type predicates, use array::first/array::last for array extraction - Remove JsonFiltering/JsonFilteringJsonPath capabilities (SurrealDB JSON access uses different syntax than SQL path operators) - Fix unwrap() → proper error propagation in wasm connector query() - Route SurrealDB adapters through new_from_external → new_surrealdb_external Schema engine improvements: - Change stubs from silent Ok(()) to explicit errors: reset(), drop_database(), introspect(), sql_schema_from_migration_history() - Fix create_migrations_table: VALUE → DEFAULT for started_at and applied_steps_count (VALUE re-evaluates on every update) - Fix renderer: DropAndRecreateColumn uses correct table name, UUID maps to native "uuid" type, BigInt→int documented, UniqueRowid→rand::ulid(), AddColumn renders DEFAULT clause properly, string escaping in defaults - Add TODO comments for acquire_lock, table_names stubs Other fixes: - Add is_surrealdb() helper on SqlFamily for API consistency - Document max_bind_values rationale (no SurrealDB limit; transport-dependent) - Add surrealdb to measure-qc-wasm-% Makefile target Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ecks - Escape single quotes in visit_raw_value for Text, Enum, Char, Json, Date, Time, Xml values to prevent SurrealQL injection - Change count_rows_in_table and count_values_in_column from Ok(0) to Ok(1) so destructive change warnings surface for populated tables Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb.rs (1)
147-154:⚠️ Potential issue | 🟠 MajorStubbed
table_namesbypasses safety checks for non-empty schemas.Returning an empty
Vecprevents detection of existing tables during initialization, which could lead to unexpected behavior. The TODO acknowledges this, but consider returning an error instead of silently returning empty results to make the limitation explicit.🐛 Proposed fix to make limitation explicit
// TODO: implement using INFO FOR DB to enumerate tables fn table_names( &mut self, _namespaces: Option<Namespaces>, _filters: SchemaFilter, ) -> BoxFuture<'_, ConnectorResult<Vec<String>>> { - Box::pin(async move { Ok(Vec::new()) }) + Box::pin(async move { + Err(ConnectorError::from_msg( + "SurrealDB table enumeration is not yet implemented. Use `prisma db push` instead.".to_owned(), + )) + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb.rs` around lines 147 - 154, The stubbed table_names implementation currently returns Ok(Vec::new()) which silently hides existing tables; change table_names (in flavour/surrealdb.rs) to return an explicit ConnectorResult error instead of an empty Vec so callers can detect the limitation—construct and return an appropriate ConnectorError (or use the crate's standard "unimplemented/unsupported" error variant) with a clear message like "table_names not implemented for SurrealDB" and include context (namespaces/filters) as needed; ensure the function signature and BoxFuture return type remain unchanged and that the error type matches other connector errors used elsewhere in this module.quaint/src/visitor/surrealdb.rs (1)
96-146:⚠️ Potential issue | 🔴 CriticalEscape string literals to prevent SurrealQL injection.
String-like values (
Text,Char,Enum,Json,Xml) are wrapped in single quotes without escaping embedded quotes. A value likeO'Reillywill generate'O'Reilly'which is invalid SurrealQL, and user-controlled data could enable injection attacks.🐛 Proposed fix to escape single quotes in string literals
- ValueType::Text(t) => t.as_ref().map(|t| self.write(format!("'{t}'"))), - ValueType::Enum(e, _) => e.as_ref().map(|e| self.write(e)), + ValueType::Text(t) => t.as_ref().map(|t| self.write(format!("'{}'", t.replace('\'', "\\'")))), + ValueType::Enum(e, _) => e.as_ref().map(|e| self.write(format!("'{}'", e.replace('\'', "\\'")))), ValueType::Bytes(b) => b.as_ref().map(|b| self.write(format!("x'{}'", hex::encode(b)))), ValueType::Boolean(b) => b.map(|b| self.write(b)), - ValueType::Char(c) => c.map(|c| self.write(format!("'{c}'"))), + ValueType::Char(c) => c.map(|c| { + let s = c.to_string(); + self.write(format!("'{}'", s.replace('\'', "\\'"))) + }),Also escape
Xmlat line 136:- ValueType::Xml(cow) => cow.as_ref().map(|cow| self.write(format!("'{cow}'"))), + ValueType::Xml(cow) => cow.as_ref().map(|cow| self.write(format!("'{}'", cow.replace('\'', "\\'")))),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quaint/src/visitor/surrealdb.rs` around lines 96 - 146, visit_raw_value wraps Text, Char, Enum, Json, and Xml values in single quotes without escaping embedded single quotes, allowing invalid SurrealQL or injection; create a small helper (e.g., escape_single_quotes) that replaces a single quote with two single quotes (or otherwise the SurrealQL-appropriate escape) and call it before formatting these variants (ValueType::Text, ::Char, ::Enum, ::Json after serde_json::to_string, and ::Xml) so you call self.write with the escaped string (e.g., self.write(format!("'{}'", escape_single_quotes(s)))) instead of raw values; update visit_raw_value to use this helper for those branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@psl/psl-core/src/builtin_connectors/surrealdb_datamodel_connector.rs`:
- Around line 110-116: The URL validation in validate_url currently treats ws://
and wss:// as first-class SurrealDB schemes; update validate_url to prefer
surrealdb:// as the canonical scheme: require surrealdb:// (and continue to
accept http:// and https://), remove or de-emphasize ws:// and wss:// as primary
matches (either reject them or accept them only as documented legacy/alternative
forms), and if you keep ws/wss support consider normalizing them to surrealdb://
or emitting a clear warning; locate and change the validate_url method to
implement this new policy and update any error message text to list surrealdb://
first and treat ws/wss as optional/legacy.
- Around line 67-74: The unreachable!() in scalar_type_for_native_type is
acceptable but please add a brief explanatory comment in the
scalar_type_for_native_type function documenting why it's unreachable: reference
that parse_native_type() always returns None,
available_native_type_constructors() is empty, and the validation in fields.rs
guards calls to scalar_type_for_native_type; this clarifies intent and
future-proofing for readers working on scalar_type_for_native_type,
parse_native_type, available_native_type_constructors, and the fields.rs
validation pipeline.
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 340-358: The fallback to "type::is_string" for JsonType::ColumnRef
in visit_json_type_equals is incorrect; instead, either resolve the ColumnRef to
a concrete JsonType before this visitor runs or explicitly handle ColumnRef here
by returning an error/unsupported result. Modify visit_json_type_equals to match
JsonType::ColumnRef and then (a) produce a visitor::Error (e.g.,
Err(visitor::Error::Unsupported("JsonType::ColumnRef not supported in SurrealDB
type check"))) or (b) build a proper dynamic comparison by visiting the column
reference expression and emitting a runtime type comparison if SurrealDB
supports it; do not silently fall back to type::is_string, and add a comment
documenting the chosen behavior. Ensure the change touches the
JsonType::ColumnRef match arm in visit_json_type_equals (and update callers that
may expect the previous fallback).
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rs`:
- Around line 115-117: get_connection_and_params currently constructs a new
Params each call and returns a reference to it; instead add a Params field to
the State struct (e.g., pub params: Params), initialize that field where State
is created, and change get_connection_and_params to return
Ok((&state.connection, &state.params)); update any State construction sites to
supply the Params so the function returns a stable reference consistent with
other connectors (symbols: get_connection_and_params, State, Params,
connection).
- Around line 90-93: The create_database function currently claims to create a
SurrealDB database but does nothing; update create_database(_state: &State) ->
ConnectorResult<String> to either execute an explicit "DEFINE DATABASE <name>"
(and optionally "DEFINE NS <ns>" if needed) via the SurrealDB client available
from State, returning success only after the command completes, or change the
function to accurately reflect that no creation is performed (e.g., return an
Err with a clear message or a status indicating no-op). Ensure you use the
existing SurrealDB client/connection in State, handle and propagate errors into
the ConnectorResult, and update the returned string/message to reflect the real
outcome.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs`:
- Around line 120-123: The match arm in the SurrealDB renderer currently ignores
TableChange::AddPrimaryKey, TableChange::DropPrimaryKey, and
TableChange::RenamePrimaryKey which lets migrations appear successful without
applying PK changes; update the renderer (the match handling those TableChange
variants) to either emit explicit SurrealDB DDL for adding/dropping/renaming
primary keys or, if SurrealDB doesn't support it, emit a clear commented
statement or a no-op notice into the generated SQL so the migration is not
silently ignored—locate the match over TableChange (where
AddPrimaryKey/DropPrimaryKey/RenamePrimaryKey are handled) and implement
explicit handling that produces an appropriate statement or comment for each
variant.
---
Duplicate comments:
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 96-146: visit_raw_value wraps Text, Char, Enum, Json, and Xml
values in single quotes without escaping embedded single quotes, allowing
invalid SurrealQL or injection; create a small helper (e.g.,
escape_single_quotes) that replaces a single quote with two single quotes (or
otherwise the SurrealQL-appropriate escape) and call it before formatting these
variants (ValueType::Text, ::Char, ::Enum, ::Json after serde_json::to_string,
and ::Xml) so you call self.write with the escaped string (e.g.,
self.write(format!("'{}'", escape_single_quotes(s)))) instead of raw values;
update visit_raw_value to use this helper for those branches.
In `@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb.rs`:
- Around line 147-154: The stubbed table_names implementation currently returns
Ok(Vec::new()) which silently hides existing tables; change table_names (in
flavour/surrealdb.rs) to return an explicit ConnectorResult error instead of an
empty Vec so callers can detect the limitation—construct and return an
appropriate ConnectorError (or use the crate's standard
"unimplemented/unsupported" error variant) with a clear message like
"table_names not implemented for SurrealDB" and include context
(namespaces/filters) as needed; ensure the function signature and BoxFuture
return type remain unchanged and that the error type matches other connector
errors used elsewhere in this module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: eabc74d9-fc09-47fc-93d4-e0b54bcc4763
📒 Files selected for processing (9)
Makefilepsl/psl-core/src/builtin_connectors/surrealdb_datamodel_connector.rsquaint/src/connector/connection_info.rsquaint/src/visitor/surrealdb.rsquery-compiler/query-compiler-wasm/src/params.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rsschema-engine/connectors/sql-schema-connector/src/lib.rs
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rs
Outdated
Show resolved
Hide resolved
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rs
Show resolved
Hide resolved
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Outdated
Show resolved
Hide resolved
… params) SurrealDB v3 rejects parameter names starting with a digit ($1, $2). Parameters must begin with a letter. Changed placeholder format from "$" + numbering to "$p" + numbering, producing $p1, $p2, etc. Verified against SurrealDB v3.0.4 Docker: - INSERT INTO user (name, email) VALUES ($p1,$p2) — works - SELECT * FROM user WHERE name = $p1 LIMIT $p2 START $p3 — works - DELETE user WHERE id = $p1 — works - UPDATE user SET name = $p1 WHERE id = $p2 — works Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 547-553: The helper function default_params unnecessarily drains
and repopulates the Vec; simplify by returning the incoming additional
Vec<Value<'_>> directly (i.e., change fn default_params(mut additional:
Vec<Value<'_>>) -> Vec<Value<'_>> to just return additional) or remove the
helper entirely and replace its call sites with an explicit vec![] where an
empty params vector is needed; update any references to default_params
accordingly.
- Around line 360-370: Replace the panic-causing unimplemented!() calls in
visit_text_search, visit_matches, and visit_text_search_relevance with returning
a proper visitor::Result error that signals full-text search is unsupported;
construct and return a descriptive visitor::Error (e.g., an UnsupportedOperation
or custom error variant) with a message like "Full-text search is not supported
on SurrealDB via this visitor" so callers (routed from FunctionType::TextSearch
and Compare::Matches/NotMatches) get a graceful error instead of a runtime
panic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 63152769-7719-4c57-a599-cadb68c16fb1
📒 Files selected for processing (2)
quaint/src/visitor/surrealdb.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/destructive_change_checker.rs
...a-engine/connectors/sql-schema-connector/src/flavour/surrealdb/destructive_change_checker.rs
Show resolved
Hide resolved
- Add explanatory comment on unreachable scalar_type_for_native_type - Remove ws:// and wss:// from validate_url (surrealdb:// is canonical) - Fix JsonType::ColumnRef to return error instead of silent fallback - Change table_names stub from Ok(Vec::new()) to explicit Err - Implement create_database with DEFINE NAMESPACE + DEFINE DATABASE - Store Params in State for stable reference in get_connection_and_params - Handle PK operations in renderer: DropPrimaryKey emits REMOVE INDEX, AddPrimaryKey/RenamePrimaryKey documented Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace unimplemented!() in visit_text_search, visit_matches, visit_text_search_relevance with proper Error returns - Simplify default_params test helper - Replace Ok(1) sentinel counts with explicit Err in count_rows_in_table and count_values_in_column (fail closed instead of fake counts) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
quaint/src/visitor/surrealdb.rs (3)
549-555: 🧹 Nitpick | 🔵 TrivialSimplify
default_paramshelper function.This function unnecessarily drains
additionalinto a newVec, which is equivalent to returningadditionaldirectly.♻️ Proposed simplification
- fn default_params(mut additional: Vec<Value<'_>>) -> Vec<Value<'_>> { - let mut result = Vec::new(); - for param in additional.drain(0..) { - result.push(param) - } - result + fn default_params(additional: Vec<Value<'_>>) -> Vec<Value<'_>> { + additional }Alternatively, remove the function entirely and use
vec![]directly in tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quaint/src/visitor/surrealdb.rs` around lines 549 - 555, The helper function default_params unnecessarily drains and clones values into a new Vec; replace its body to simply return the incoming Vec<Value<'_>> (i.e., return additional) or remove the function and replace call sites with vec![] where appropriate; update references to default_params in the module so they use the direct Vec without draining to avoid the extra allocation and preserve ownership of Value<'_>.
352-352:⚠️ Potential issue | 🟡 Minor
JsonType::ColumnReffallback totype::is_stringmay produce incorrect results.Line 352 falls back to
type::is_stringforColumnRefvariants. This is semantically incorrect for dynamic type comparison—if the referenced column contains a non-string JSON type, this always returns false.
362-372:⚠️ Potential issue | 🟠 MajorReplace
unimplemented!()with proper error returns to avoid runtime panics.The
visit_text_search,visit_matches, andvisit_text_search_relevancemethods useunimplemented!()which will panic at runtime. Since these are required trait methods (per context snippet 3), they are reachable via query compilation. Return a descriptive error instead.🐛 Proposed fix to return errors instead of panicking
fn visit_text_search(&mut self, _text_search: crate::prelude::TextSearch<'a>) -> visitor::Result { - unimplemented!("Full-text search is not yet supported on SurrealDB via this visitor") + Err(Error::builder(ErrorKind::conversion( + "Full-text search is not yet supported for SurrealDB" + )).build()) } fn visit_matches(&mut self, _left: Expression<'a>, _right: Expression<'a>, _not: bool) -> visitor::Result { - unimplemented!("Full-text search is not yet supported on SurrealDB via this visitor") + Err(Error::builder(ErrorKind::conversion( + "Full-text search MATCHES is not yet supported for SurrealDB" + )).build()) } fn visit_text_search_relevance(&mut self, _text_search_relevance: TextSearchRelevance<'a>) -> visitor::Result { - unimplemented!("Full-text search is not yet supported on SurrealDB via this visitor") + Err(Error::builder(ErrorKind::conversion( + "Full-text search relevance is not yet supported for SurrealDB" + )).build()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quaint/src/visitor/surrealdb.rs` around lines 362 - 372, The three visitor methods visit_text_search, visit_matches, and visit_text_search_relevance currently call unimplemented!() which will panic at runtime; replace each panic with returning an Err of the visitor::Result that contains a descriptive error (e.g. "Full-text search is not supported on SurrealDB" plus the method name and any relevant context) so compilation/consumers get a recoverable error instead of a panic; use the crate's visitor::Error/constructor available in this module to build and return that error from each of the three functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 228-233: The current branch that handles insert.on_conflict
(matching OnConflict::Update) emits PostgreSQL-style "ON CONFLICT ..." which
SurrealDB doesn't support; update the logic in surrealdb.rs where
insert.on_conflict is matched (the block that calls self.write(" ON CONFLICT "),
self.columns_to_bracket_list(constraints), and self.visit_upsert(update)) to
instead emit a SurrealDB-compatible upsert: detect OnConflict::Update(update,
constraints) and translate it into a SurrealDB "UPSERT" form (use the constraint
columns as the upsert key/WHERE predicate and convert the update clause into SET
assignments) or, if your codebase supports multiple SurrealDB versions, branch
to emit the v1.x "INSERT ... ON DUPLICATE KEY UPDATE" or v2.x "UPSERT ... SET
..." equivalents; ensure you remove the PostgreSQL "ON CONFLICT" emission and
map visit_upsert/update nodes into SurrealDB SET semantics (or raise a clear
unsupported-operation error if you cannot map the pattern).
- Around line 529-536: The escape_squote function currently only escapes single
quotes, leaving backslashes unescaped which allows inputs like `foo\'--` to
break out of the string; update escape_squote so it first escapes backslashes
then escapes single quotes (e.g. replace "\" with "\\\\" then "'" with "\\'")
and return Cow::Owned when any replacement occurs (otherwise Cow::Borrowed),
keeping the function name escape_squote and behavior for SurrealQL string
literals intact.
---
Duplicate comments:
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 549-555: The helper function default_params unnecessarily drains
and clones values into a new Vec; replace its body to simply return the incoming
Vec<Value<'_>> (i.e., return additional) or remove the function and replace call
sites with vec![] where appropriate; update references to default_params in the
module so they use the direct Vec without draining to avoid the extra allocation
and preserve ownership of Value<'_>.
- Around line 362-372: The three visitor methods visit_text_search,
visit_matches, and visit_text_search_relevance currently call unimplemented!()
which will panic at runtime; replace each panic with returning an Err of the
visitor::Result that contains a descriptive error (e.g. "Full-text search is not
supported on SurrealDB" plus the method name and any relevant context) so
compilation/consumers get a recoverable error instead of a panic; use the
crate's visitor::Error/constructor available in this module to build and return
that error from each of the three functions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b071572-f2ee-41cf-9bd2-d5d465d6b46f
📒 Files selected for processing (1)
quaint/src/visitor/surrealdb.rs
Replace all "not implemented" stubs with real SurrealDB implementations: **Introspection** (introspect): - Queries INFO FOR DB to list tables - Queries INFO FOR TABLE for each table to get fields - Parses DEFINE FIELD definitions to extract type, nullability - Builds SqlSchema with proper Column/ColumnType/ColumnArity **Table enumeration** (list_tables / table_names): - Parses INFO FOR DB response to extract table names from "tables" object **Row/column counting** (count_rows_in_table / count_values_in_column): - Executes SELECT count() FROM table GROUP ALL - Executes SELECT count() FROM table WHERE column IS NOT NONE GROUP ALL **Database lifecycle** (create_database / drop_database / reset): - create_database: DEFINE NAMESPACE + DEFINE DATABASE - drop_database: REMOVE DATABASE IF EXISTS - reset: list all tables via INFO FOR DB, then REMOVE TABLE each **Other fixes from CodeRabbit round 4:** - Replace unimplemented!() with Error returns in text search visitors - Simplify default_params test helper Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace PostgreSQL-style ON CONFLICT DO with SurrealDB-compatible ON DUPLICATE KEY UPDATE for upsert operations - Fix escape_squote to escape backslashes before single quotes, preventing injection via sequences like \' Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
acquire_lock: Implements advisory locking via UPSERT on _prisma_lock:migration_lock record. Creates lock table if needed, upserts lock row with timestamp. sql_schema_from_migration_history: Full implementation that replays migration scripts in order against the connection, then introspects the resulting schema via INFO FOR DB / INFO FOR TABLE. No TODOs, FIXMEs, stubs, or "not implemented" messages remain in any SurrealDB file across both repositories. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 24 new tests covering: - SELECT with ORDER BY, field selection, aliasing, DISTINCT - WHERE conditions: AND, OR, NOT, IN, IS NULL, IS NOT NULL, LIKE - JOIN: INNER JOIN, LEFT JOIN - INSERT: default values, ON DUPLICATE KEY UPDATE, RETURN clause, comments - UPDATE: with and without WHERE - DELETE: with WHERE and without (delete all) - Raw values: null, int, boolean, text with quotes, text with backslash - Pagination: LIMIT + START Total: 7 original + 24 new = 31 passing tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
quaint/src/visitor/surrealdb.rs (2)
538-545:⚠️ Potential issue | 🔴 CriticalEscape backslashes before single quotes.
SurrealDB string parsing treats backslash as an escape introducer; single-quoted parsing supports
\\and\', and the v2 parser still validates several backslash escapes explicitly. Replacing only'leaves raw backslashes in the emitted literal, so inputs containing\can be retokenized or truncated after wrapping with single quotes. (github.com)🛠️ Escape `\` first, then `'`
fn escape_squote(s: &str) -> Cow<'_, str> { - if s.contains('\'') { - Cow::Owned(s.replace('\'', "\\'")) + if s.contains('\\') || s.contains('\'') { + Cow::Owned(s.replace('\\', "\\\\").replace('\'', "\\'")) } else { Cow::Borrowed(s) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quaint/src/visitor/surrealdb.rs` around lines 538 - 545, The escape_squote function currently only replaces single quotes, which leaves raw backslashes that can break SurrealQL parsing; update escape_squote to first escape backslashes (replace '\' with "\\\\") and then escape single quotes (replace '\'' with "\\'"), returning a Cow::Owned only when replacements occur so inputs without backslashes or quotes still return Cow::Borrowed; reference function name escape_squote to locate and modify the implementation accordingly.
228-233:⚠️ Potential issue | 🟠 MajorStop emitting PostgreSQL
ON CONFLICTsyntax here.SurrealDB
INSERTsupportsON DUPLICATE KEY UPDATE, and its dedicated upsert form isUPSERT;ON CONFLICT (...) DO ...is not valid SurrealQL. As written, this branch makesNativeUpsertcompile into an invalid statement. (surrealdb.com)🛠️ Safer stop-gap
- if let Some(OnConflict::Update(update, constraints)) = insert.on_conflict { - self.write(" ON CONFLICT ")?; - self.columns_to_bracket_list(constraints)?; - self.write(" DO ")?; - self.visit_upsert(update)?; + if let Some(OnConflict::Update(_, _)) = insert.on_conflict { + return Err(Error::builder(ErrorKind::conversion( + "SurrealDB upsert translation is not implemented; use ON DUPLICATE KEY UPDATE or UPSERT syntax", + )) + .build()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quaint/src/visitor/surrealdb.rs` around lines 228 - 233, The current branch that handles insert.on_conflict emits PostgreSQL's "ON CONFLICT (...) DO ..." which is invalid SurrealQL; update the logic in the branch that matches OnConflict::Update(update, constraints) so it stops emitting the conflict target and PostgreSQL keywords: remove the call to columns_to_bracket_list(constraints) and the " ON CONFLICT " / " DO " sequence, and instead write the SurrealDB form (use "ON DUPLICATE KEY UPDATE" or emit an UPSERT statement depending on how NativeUpsert should map) before calling self.visit_upsert(update); ensure you update the code paths in the match arm that currently reference insert.on_conflict, OnConflict::Update, columns_to_bracket_list, and self.visit_upsert to produce valid SurrealQL.schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs (1)
232-238:⚠️ Potential issue | 🟠 MajorA comment-only rename script will succeed as a no-op.
This returns a SQL comment, not a failing statement. If a rename migration reaches this renderer, the migration can report success while leaving the old table name in place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs` around lines 232 - 238, The current render_rename_table returns a SQL comment which is a no-op; change it to return a statement that will deliberately fail the migration instead (so the migration cannot silently succeed). In the render_rename_table function, replace the commented format string with a deliberately-invalid SQL statement containing the error message (e.g. emit a custom token like "RAISE_ERROR 'SurrealDB does not support RENAME TABLE: old -> new';") so any SQL executor/parsing will error; keep the message including name and new_name to aid debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 104-105: The visitor currently emits raw quoted strings or invalid
hex bytes (ValueType::Bytes) and doesn't use SurrealQL typed literals for UUIDs
and datetimes; update the match arms for ValueType::Uuid and ValueType::DateTime
to emit typed literals using the u"..." and d"..." prefixes respectively, and
change ValueType::Bytes handling to emit a bytes constructor SurrealQL
understands (e.g., encode the bytes as base64 and emit a cast or constructor
such as <bytes>"BASE64_DATA" or call encoding::base64::decode("BASE64_DATA") in
the generated SQL) instead of x'...'; adjust the code paths in the visitor where
ValueType::Bytes, ValueType::Uuid and ValueType::DateTime are handled so the
writer writes the correct prefixed literals or bytes constructor.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rs`:
- Around line 131-143: create_database and drop_database are hardcoded to the
"prisma" namespace/database; change them to use the actual namespace/database
from the current State instead of the literal "prisma". In the create_database
and drop_database functions, obtain the namespace and database identifiers from
State (e.g., from state or state.connection metadata that represents the opened
datasource) and interpolate those values into the raw_cmd calls used (instead of
the hardcoded "prisma"), ensuring you properly escape/validate the identifiers
before building the DEFINE/REMOVE statements so commands target the correct
datasource and avoid injection.
- Around line 208-218: The parser in mod.rs currently maps the string "int" only
to ColumnTypeFamily::Int, which collapses BigInt round-trips; instead, update
the renderer (render_column_type in
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs)
to emit a distinct "bigint" token for ColumnTypeFamily::BigInt, and then update
the match in this file (the match on type_str in mod.rs) to map "bigint" =>
ColumnTypeFamily::BigInt while keeping "int" => ColumnTypeFamily::Int so BigInt
columns survive introspect/drift cycles.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs`:
- Around line 123-126: The PK index name is being quoted incorrectly by
formatting "{table_name}_pk" around self.quote(tables.previous.name()),
producing identifiers like `users`_pk; update both the drop and create/redefine
paths (e.g., the TableChange::DropPrimaryKey branch and the corresponding
create/redefine branch around the later occurrence) to build the full PK name
first (let pk_name = format!("{}_pk", tables.previous.name()) or using the
appropriate table variant), then call self.quote(&pk_name) so the entire
identifier (e.g., users_pk) is quoted as a single token before inserting into
the DDL statements.
- Around line 97-118: The DropAndRecreateColumn branch for
TableChange::DropAndRecreateColumn currently rebuilds type and nullability but
omits the next column's default, which removes defaults during migration; update
the logic in that branch (where previous_column and next_column are fetched and
col_type/nullable are computed) to read next_column.default(), format it into a
SurrealDB DEFINE FIELD clause (e.g., append " DEFAULT <default_expr>" when
Some), and include that appended default string in the second statements.push
format so the recreated field preserves next_column.default().
- Around line 215-227: The current render_redefine_tables implementation issues
a destructive "REMOVE TABLE" which causes data loss; update
render_redefine_tables (and related logic using RedefineTable and
schemas.walk/MigrationPair) to perform a safe rename/copy workflow: for each
redefine_table, create a temporary table using render_create_table for
tables.next (use a unique temp name derived from quote(tables.previous.name())),
copy rows from the old table into the temp table mapping only existing column
names (generate an INSERT INTO <temp> SELECT <cols> FROM <old>), drop the old
table, rename the temp table to the original table name, and then recreate
non-PK indexes with render_create_index on the final table; ensure column
mapping handles added/removed/renamed columns and that quoted identifiers from
quote(...) are used consistently.
---
Duplicate comments:
In `@quaint/src/visitor/surrealdb.rs`:
- Around line 538-545: The escape_squote function currently only replaces single
quotes, which leaves raw backslashes that can break SurrealQL parsing; update
escape_squote to first escape backslashes (replace '\' with "\\\\") and then
escape single quotes (replace '\'' with "\\'"), returning a Cow::Owned only when
replacements occur so inputs without backslashes or quotes still return
Cow::Borrowed; reference function name escape_squote to locate and modify the
implementation accordingly.
- Around line 228-233: The current branch that handles insert.on_conflict emits
PostgreSQL's "ON CONFLICT (...) DO ..." which is invalid SurrealQL; update the
logic in the branch that matches OnConflict::Update(update, constraints) so it
stops emitting the conflict target and PostgreSQL keywords: remove the call to
columns_to_bracket_list(constraints) and the " ON CONFLICT " / " DO " sequence,
and instead write the SurrealDB form (use "ON DUPLICATE KEY UPDATE" or emit an
UPSERT statement depending on how NativeUpsert should map) before calling
self.visit_upsert(update); ensure you update the code paths in the match arm
that currently reference insert.on_conflict, OnConflict::Update,
columns_to_bracket_list, and self.visit_upsert to produce valid SurrealQL.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs`:
- Around line 232-238: The current render_rename_table returns a SQL comment
which is a no-op; change it to return a statement that will deliberately fail
the migration instead (so the migration cannot silently succeed). In the
render_rename_table function, replace the commented format string with a
deliberately-invalid SQL statement containing the error message (e.g. emit a
custom token like "RAISE_ERROR 'SurrealDB does not support RENAME TABLE: old ->
new';") so any SQL executor/parsing will error; keep the message including name
and new_name to aid debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a983f63-e121-4a01-89b4-2e0de1a0ac6a
📒 Files selected for processing (6)
psl/psl-core/src/builtin_connectors/surrealdb_datamodel_connector.rsquaint/src/visitor/surrealdb.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/destructive_change_checker.rsschema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rs
Show resolved
Hide resolved
Comment on lines
+208
to
+218
| let family = match type_str { | ||
| "bool" => ColumnTypeFamily::Boolean, | ||
| "int" => ColumnTypeFamily::Int, | ||
| "float" => ColumnTypeFamily::Float, | ||
| "decimal" => ColumnTypeFamily::Decimal, | ||
| "string" => ColumnTypeFamily::String, | ||
| "datetime" => ColumnTypeFamily::DateTime, | ||
| "bytes" => ColumnTypeFamily::Binary, | ||
| "object" | "record" => ColumnTypeFamily::Json, | ||
| "uuid" => ColumnTypeFamily::Uuid, | ||
| _ => ColumnTypeFamily::String, |
There was a problem hiding this comment.
Introspection collapses BigInt fields to Int.
render_column_type() in schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs maps both ColumnTypeFamily::Int and ColumnTypeFamily::BigInt to int, but this parser maps every TYPE int back to ColumnTypeFamily::Int. BigInt columns will therefore come back from db pull / drift detection as Int.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/connector/wasm/mod.rs`
around lines 208 - 218, The parser in mod.rs currently maps the string "int"
only to ColumnTypeFamily::Int, which collapses BigInt round-trips; instead,
update the renderer (render_column_type in
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs)
to emit a distinct "bigint" token for ColumnTypeFamily::BigInt, and then update
the match in this file (the match on type_str in mod.rs) to map "bigint" =>
ColumnTypeFamily::BigInt while keeping "int" => ColumnTypeFamily::Int so BigInt
columns survive introspect/drift cycles.
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Outdated
Show resolved
Hide resolved
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Show resolved
Hide resolved
schema-engine/connectors/sql-schema-connector/src/flavour/surrealdb/renderer.rs
Show resolved
Hide resolved
…oper quoting
Visitor (quaint):
- UUID raw values use SurrealQL u"..." typed literal prefix
- DateTime raw values use d"..." typed literal prefix
- Bytes use encoding::base64::decode('...') instead of invalid x'...'
Schema engine renderer:
- BigInt emits "bigint" (not "int") for round-trip fidelity
- Parser maps "bigint" → ColumnTypeFamily::BigInt
- DropAndRecreateColumn preserves column defaults
- PK index names quoted as single token (users_pk not `users`_pk)
- render_rename_table emits THROW (fails migration) not comment
- render_redefine_tables uses safe copy-and-swap workflow:
create temp → INSERT INTO temp SELECT FROM old → drop old →
create final → INSERT INTO final SELECT FROM temp → drop temp
Wasm connector:
- create_database/drop_database use actual ns/db from State
- State stores namespace/database fields
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full SurrealDB integration across the Rust codebase — PSL parser, query compiler with SurrealQL visitor, and schema engine with DDL generation.
Flavour::SurrealDb,SurrealDbDatamodelConnector(Connector trait, no native types, no FK constraints)SqlFamily::SurrealDb, SurrealQL visitor with$1/$2placeholders,DELETEwithoutFROM,LIMIT ... START,RETURNinstead ofRETURNING,string::concat()SurrealDbDialect,SurrealDbConnector(external adapter only),SurrealDbRenderer(DEFINE TABLE SCHEMAFULL,DEFINE FIELD ON TABLE TYPE,DEFINE INDEX,REMOVE TABLE/FIELD/INDEX), introspection flavour, destructive change checker, schema calculator, schema differFiles changed (25 modified + 10 new)
PSL (3 files):
psl/psl-core/Cargo.toml,psl/psl/Cargo.toml— feature flagpsl/psl-core/src/datamodel_connector.rs—Flavour::SurrealDbpsl/psl-core/src/builtin_connectors/— connector registration + implementationQuaint (6 files):
quaint/Cargo.toml,quaint/src/lib.rs— feature flag + compile_error guardquaint/src/connector/connection_info.rs—SqlFamily::SurrealDbwith all match armsquaint/src/connector/external.rs—AdapterProvider::SurrealDbquaint/src/visitor/surrealdb.rs— SurrealQL visitor (~540 lines, 7 tests)Query Compiler (4 files):
query-compiler/query-compiler/— feature + dispatchquery-compiler/query-compiler-wasm/— feature + params + build.shSchema Engine (12 files):
src/flavour/surrealdb/— full flavour implementation (7 files)Test plan
cargo checkpasses for all 6 affected crates with--features surrealdbcargo test -p quaint --features surrealdb -- surrealdb— 7/7 visitor tests passcargo testwith all featuresmake build-qc-wasmCompanion PR: prisma/prisma#29367
🤖 Generated with Claude Code
Summary by CodeRabbit