orm: fix u8/i8 OID mapping and string default quoting - #27989
Open
Jengro777 wants to merge 12 commits into
Open
Conversation
Fix vlang#27986: Map u8 to Oid.t_int2 (smallint) instead of Oid.t_char in pg_stmt_match, using conv.hton16 for proper byte order conversion. Previously u8 was bound as PostgreSQL's internal "char" type, causing type mismatch errors when the column was smallint. Fix vlang#27987: Properly quote string values in DEFAULT clauses during DDL generation. Track whether the default came from a string attribute and wrap with SQL single quotes to avoid invalid SQL like DEFAULT /dashboard instead of DEFAULT '/dashboard'. Co-Authored-By: Claude <noreply@anthropic.com>
Member
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6426f7699e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…string-default-quoting
Add is_sql_expr() helper to detect SQL function calls (e.g. gen_random_uuid()) and keyword constants (e.g. CURRENT_TIME, CURRENT_DATE, CURRENT_TIMESTAMP) so that @[default: 'CURRENT_TIME'] correctly produces DEFAULT CURRENT_TIME rather than DEFAULT 'CURRENT_TIME'. String literal defaults (e.g. @[default: '/dashboard']) continue to be properly quoted with single quotes. Co-Authored-By: Claude <noreply@anthropic.com>
…istic Replace the all-uppercase heuristic with a clean kind-based strategy: - .plain kind (bare identifier, e.g. @[default: CURRENT_TIME]) → no quote - .number kind (e.g. @[default: 42]) → no quote - .string kind with parentheses (e.g. @[default: 'gen_random_uuid()']) → no quote - .string kind otherwise (e.g. @[default: '/dashboard']) → quote This eliminates false positives where uppercase string values like 'YES', 'ACTIVE', or 'NULL' would be incorrectly treated as SQL expressions. Users who need SQL keyword defaults should use bare identifiers (without quotes) — which V's attribute parser already supports as .plain kind. Updated sqlite_orm_test.v to use bare identifiers for CURRENT_TIME, CURRENT_DATE, and CURRENT_TIMESTAMP. Co-Authored-By: Claude <noreply@anthropic.com>
Replace @[default: 'CURRENT_TIMESTAMP'] with @[default: CURRENT_TIMESTAMP] (bare identifier, .plain kind) so the SQL expression is not quoted. This aligns with the kind-based approach: .plain = SQL expression (no quote), .string = string literal (quote). Covered databases: - PostgreSQL: CURRENT_TIMESTAMP, gen_random_uuid() (parens detected) - MySQL: CURRENT_TIMESTAMP - SQLite: CURRENT_TIME, CURRENT_DATE, CURRENT_TIMESTAMP (already updated) Co-Authored-By: Claude <noreply@anthropic.com>
…xpressions Update all remaining uses of quoted SQL expression defaults across the codebase to use bare identifiers (.plain kind): - vlib/orm/orm_option_time_test.v: CURRENT_TIME - vlib/v/tests/orm_array_field_test.v: CURRENT_TIME - vlib/v3/tests/orm_join_sql_attr_test.v: db_default (string literal) - examples/veb_orm_jwt/user_entities.v: CURRENT_TIMESTAMP (x3) - examples/veb_fullstack/product_entities.v: CURRENT_TIMESTAMP Also fix the old manual double-quote workaround in v3 test. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
request for examination |
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 933c5c62de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…string-default-quoting # Conflicts: # vlib/v3/tests/orm_join_sql_attr_test.v
… defaults
- is_sql_expr(): require the part before '(' to be a valid SQL identifier
(starts with letter/underscore, contains only letters/digits/underscores).
This prevents strings like 'pending (manual)' from being misclassified
as SQL expressions.
- Escape single quotes in string default values (' -> '') to produce
valid SQL DDL when values contain apostrophes (e.g. Bob's account).
Co-Authored-By: Claude <noreply@anthropic.com>
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 780851ad19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The write side correctly sends u8 as SMALLINT via hton16, but val_to_primitive still parsed the returned text with str.i8(), which rejects values above 127 and silently falls back to 0. Parse with str.i16() and cast to u8 instead. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
…string-default-quoting
…m:Jengro777/v into fix/orm-u8-oid-and-string-default-quoting
is_sql_expr() now also treats paren-less SQL keywords like CURRENT_TIMESTAMP / CURRENT_DATE as expressions, so string-form @[default: 'CURRENT_TIMESTAMP'] keeps emitting unquoted DEFAULT clauses instead of silently becoming string literals. Update the orm README to document the default quoting rules and switch examples to bare identifiers. Add a local DDL-generation test covering function-call defaults, keyword defaults, apostrophe escaping and paren-containing strings; MockDB gains capture_only mode for tests that only assert the generated SQL.
Contributor
Author
|
@copilot review |
Contributor
Author
|
@GGRei |
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.
Fix #27986 —
u8andi8bound as wrong PostgreSQL typeBefore:
u8andi8were bound asOid.t_char(PostgreSQL internal"char"type, OID 18), causing:After: Both map to
Oid.t_int2(smallint) with properconv.hton16()byte-order conversion, consistent withi16/u16.u8t_char(18)t_int2(21)conv.hton16(u16(data))i8t_char(18)t_int2(21)conv.hton16(u16(data))Values verify correctly:
u8(255)→u16(255)→ int2 reads 255 ✅i8(-1)→u16(65535)→ int2 reads -1 ✅ (matchesi16pattern)Fix #27987 — String
default:values not quoted in DDLBefore:
@[default: '\''/dashboard'\'']generated invalid SQL:After: String defaults are properly quoted:
Numeric defaults remain unquoted (
DEFAULT 0,DEFAULT 33), and empty string defaults (DEFAULT '\'''\'') continue to work via the existing explicit branch.Breaking change: The
@[default: '\''"...'\'']workaround (embedding manual double-quotes) is no longer needed. Tests updated accordingly.Files Changed
vlib/db/pg/orm.vu8/i8:t_char→t_int2withhton16vlib/orm/orm.vvlib/orm/orm_func_test.v"..."workaroundvlib/orm/orm_null_test.vvlib/orm/orm_upsert_test.v"..."workaroundTests
All 38 ORM tests pass.