feat(sql): implement TypeID specification 0.3.0 in typeid-sql #537
Conversation
joschi
force-pushed
the
typeid-sql-spec-0.3
branch
2 times, most recently
from
be313e1 to
341d77b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements TypeID specification 0.3.0 in the typeid-sql project, updating the prefix validation rules to allow underscores within prefixes while maintaining proper formatting constraints.
- Updates prefix regex from
[a-z]{0,63}to^([a-z]([a-z_]{0,61}[a-z])?)?$to allow underscores - Adds comprehensive test coverage for underscore handling in prefixes
- Includes testing infrastructure and documentation improvements
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| typeid/typeid-sql/sql/03_typeid.sql | Updates core TypeID functions with new prefix validation regex and improves parsing logic |
| typeid/typeid-sql/supabase/tests/03_typeid.test.sql | Adds tests for underscore handling and updates error message expectations |
| typeid/typeid-sql/supabase/tests/03_typed_text.test.sql | Adds underscore-specific test cases and updates validation error messages |
| typeid/typeid-sql/package.json | Adds npm scripts for test automation |
| typeid/typeid-sql/README.md | Documents testing procedures and requirements |
Comments suppressed due to low confidence (2)
typeid/typeid-sql/supabase/tests/03_typeid.test.sql:128
- The test description 'Print valid: valid-alphabet-underscores' is inconsistent with the test name 'valid-prefix-underscores'. Consider using 'Print valid: valid-prefix-underscores' for consistency.
'Print valid: valid-alphabet-underscores'
typeid/typeid-sql/supabase/tests/03_typeid.test.sql:133
- The test description 'Print valid: valid-alphabet-underscores' should be more specific since this test uses 'pre_____fix' with multiple consecutive underscores. Consider 'Print valid: valid-prefix-multiple-underscores'.
'Print valid: valid-alphabet-underscores'
| begin | ||
| if (typeid_str is null) then | ||
| return null; | ||
| end if; | ||
| if position('_' in typeid_str) = 0 then | ||
| return ('', base32_decode(typeid_str))::typeid; | ||
| end if; | ||
| prefix = split_part(typeid_str, '_', 1); | ||
| suffix = split_part(typeid_str, '_', 2); | ||
| matches = regexp_match(typeid_str, '^(.*)_(.*)$'); |
There was a problem hiding this comment.
The regex ^(.*)_(.*)$ uses greedy matching which could be inefficient for strings with multiple underscores. Consider using ^(.*?)_(.*)$ with non-greedy matching or ^([^_]+)_(.*)$ to match only the first underscore more efficiently.
| matches = regexp_match(typeid_str, '^(.*)_(.*)$'); | |
| matches = regexp_match(typeid_str, '^([^_]+)_(.*)$'); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@loreto WDYT? We could already match the correct regular expression here (prefix + suffix) but would lose fidelity in the error messages afterwards.
joschi
force-pushed
the
typeid-sql-spec-0.3
branch
from
341d77b to
18fca9c
Compare
|
Apologies for the delay. I'll take a look at this PR today. |
Summary
Implement TypeID specification 0.3.0 in
typeid-sql.Note
PLEASE tag the repository with the specification and provide a changelog. 🙏
How was it tested?
yarn install yarn testCommunity Contribution License
All community contributions in this pull request are licensed to the project maintainers under the terms of the Apache 2 License.
By creating this pull request I represent that I have the right to license the contributions to the project maintainers under the Apache 2 License as stated in the Community Contribution License.