Add multibyte search_path case conversion coverage#1206
Add multibyte search_path case conversion coverage#1206gaoxueyu merged 1 commit intoIvorySQL:masterfrom
Conversation
Extend case_conversion.sql to cover multibyte quoted schema names through the search_path code path. The scenario now makes the active identifier_case_switch setting explicit, checks both interchange and normal behavior, and includes a failure case where a quoted schema name stops resolving once the case-conversion mode changes. Also removed the alternative file output, I suppose it was added by mistake because their diff was very small.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughUpdates regression test fixtures for identifier case conversion feature, removing previous expectations from case_conversion_1.out and expanding case_conversion.out with new schema-based test sequences using Chinese character identifiers. Adds corresponding SQL test setup blocks in case_conversion.sql to exercise identifier_case_switch modes across schema and search_path operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/utils/adt/varlena.c`:
- Around line 2757-2766: The code in identifier_case_transform_cstring wrongly
asserts that identifier_case_transform returns the same byte length; change the
check to allow a shorter result and copy only the transformed bytes plus the NUL
terminator instead of assuming equal length. Concretely, replace the
Assert(strlen(new_name) == len) with Assert(strlen(new_name) <= len) (or
equivalent) and use the transformed length (strlen(new_name)) when copying back
into ident (e.g. memcpy/strncpy of strlen(new_name)+1) so truncated names
produced by identifier_case_transform (via
upcase_identifier()/downcase_identifier()) are handled safely before
pfree(new_name).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 502e88fc-d75d-48e9-89e7-473146567e34
📒 Files selected for processing (4)
src/backend/utils/adt/varlena.csrc/oracle_test/regress/expected/case_conversion.outsrc/oracle_test/regress/expected/case_conversion_1.outsrc/oracle_test/regress/sql/case_conversion.sql
💤 Files with no reviewable changes (1)
- src/oracle_test/regress/expected/case_conversion_1.out
cb1b0df to
be7f6e4
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Extend case_conversion.sql to cover multibyte quoted schema names through the search_path code path. The scenario now makes the active identifier_case_switch setting explicit, checks both interchange and normal behavior, and includes a failure case where a quoted schema name stops resolving once the case-conversion mode changes.
Also removed the alternative file output, I suppose it was added by mistake because their diff was very small.
Summary by CodeRabbit