Conversation
WalkthroughIntroduces a .coderabbit.yaml config, adds a code-format target in GNUmakefile.in, removes enable-git-hooks from Makefile, updates README instructions to use make code-format, fixes comment text/spacing in SQL and C sources, removes dead #if 0 GUC code, and repositions Ivy GUC blocks in guc_tables.c. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Make as make (GNUmakefile.in)
participant Script as tools/enable-git-hooks.sh
participant Git as Git Hooks
Dev->>Make: make code-format
Make->>Make: submake-generated-headers
Make->>Script: bash tools/enable-git-hooks.sh
Script->>Git: Install/update pre-commit hook
Git-->>Dev: Hook ready for formatting on commits
note over Dev,Git: Target name changed from enable-git-hooks to code-format
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 0
🧹 Nitpick comments (2)
contrib/ivorysql_ora/src/datatype/oratimestamp.c (1)
225-225: Consider improving error message specificity.The error message "the precision of datetime out of range" could be more helpful by specifying the valid range (e.g., "TIMESTAMP precision must be between 0 and 9").
Apply this diff to improve clarity:
- errmsg("the precision of datetime out of range"))); + errmsg("timestamp precision out of range (must be 0-9)")));contrib/ivorysql_ora/src/datatype/oratimestampltz.c (1)
137-137: Consider improving error message specificity.Same as the suggestion for oratimestamp.c line 225 - the error message could be more descriptive about the valid range.
Apply this diff:
- errmsg("the precision of datetime out of range"))); + errmsg("timestamp precision out of range (must be 0-9)")));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
contrib/ivorysql_ora/expected/ora_datetime.outis excluded by!**/*.outcontrib/ivorysql_ora/expected/ora_datetime_datatype_functions.outis excluded by!**/*.outsrc/oracle_test/regress/expected/expressions.outis excluded by!**/*.out
📒 Files selected for processing (14)
.coderabbit.yaml(1 hunks)GNUmakefile.in(1 hunks)Makefile(0 hunks)README.md(1 hunks)README_CN.md(1 hunks)contrib/ivorysql_ora/sql/ora_datetime.sql(2 hunks)contrib/ivorysql_ora/src/builtin_functions/datetime_datatype_functions.c(72 hunks)contrib/ivorysql_ora/src/datatype/oradate.c(4 hunks)contrib/ivorysql_ora/src/datatype/oratimestamp.c(25 hunks)contrib/ivorysql_ora/src/datatype/oratimestampltz.c(6 hunks)contrib/ivorysql_ora/src/datatype/oratimestamptz.c(7 hunks)contrib/ivorysql_ora/src/guc/guc.c(2 hunks)src/backend/utils/misc/guc_tables.c(7 hunks)src/backend/utils/misc/ivy_guc.c(15 hunks)
💤 Files with no reviewable changes (1)
- Makefile
🧰 Additional context used
🧬 Code graph analysis (6)
src/backend/utils/misc/ivy_guc.c (1)
src/backend/utils/error/elog.c (3)
errcode(853-864)errmsg(1070-1086)errhint(1317-1332)
contrib/ivorysql_ora/src/datatype/oratimestamptz.c (5)
src/backend/utils/adt/timestamp.c (1)
timestamptz_in(417-481)src/include/postgres.h (4)
CStringGetDatum(354-358)ObjectIdGetDatum(256-260)Int32GetDatum(216-220)PointerGetDatum(326-330)src/backend/utils/adt/formatting.c (1)
timestamptz_to_char(4667-4712)src/include/utils/timestamp.h (1)
TimestampTzGetDatum(51-55)src/backend/utils/adt/varlena.c (1)
cstring_to_text(193-197)
contrib/ivorysql_ora/src/datatype/oratimestampltz.c (5)
src/backend/utils/error/elog.c (2)
errmsg(1070-1086)errcode(853-864)src/backend/utils/adt/formatting.c (1)
timestamptz_to_char(4667-4712)src/include/utils/timestamp.h (1)
TimestampTzGetDatum(51-55)src/include/postgres.h (1)
PointerGetDatum(326-330)src/backend/utils/adt/varlena.c (1)
cstring_to_text(193-197)
contrib/ivorysql_ora/src/datatype/oradate.c (4)
src/backend/utils/adt/date.c (2)
date_timestamp(1347-1356)date_in(113-179)src/include/postgres.h (2)
CStringGetDatum(354-358)PointerGetDatum(326-330)src/include/utils/timestamp.h (1)
TimestampGetDatum(45-49)src/backend/utils/adt/varlena.c (1)
cstring_to_text(193-197)
contrib/ivorysql_ora/src/datatype/oratimestamp.c (4)
src/backend/utils/adt/timestamp.c (1)
timestamp_in(165-228)src/include/postgres.h (4)
CStringGetDatum(354-358)ObjectIdGetDatum(256-260)Int32GetDatum(216-220)PointerGetDatum(326-330)src/include/utils/timestamp.h (2)
TimestampGetDatum(45-49)IntervalPGetDatum(57-61)src/backend/utils/adt/varlena.c (1)
cstring_to_text(193-197)
contrib/ivorysql_ora/src/builtin_functions/datetime_datatype_functions.c (7)
src/backend/utils/adt/timestamp.c (2)
tm2timestamp(2005-2038)timestamp2tm(1909-1995)src/backend/access/transam/xact.c (1)
GetCurrentTransactionStartTimestamp(869-873)contrib/ivorysql_ora/src/datatype/oratimestamp.c (1)
OraAdjustTimestampForTypmod(134-187)src/backend/utils/adt/datetime.c (1)
DetermineTimeZoneOffset(1649-1655)src/backend/utils/adt/varlena.c (2)
text_to_cstring(226-242)cstring_to_text(193-197)src/timezone/localtime.c (1)
pg_get_timezone_name(1988-1994)src/backend/utils/adt/formatting.c (2)
timestamp_to_char(4630-4665)timestamptz_to_char(4667-4712)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: contrib_regression
- GitHub Check: meson_build (ubuntu-latest)
- GitHub Check: oracle_regression (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: oracle_pg_regression (ubuntu-latest)
- GitHub Check: pg_regression (ubuntu-latest)
🔇 Additional comments (22)
GNUmakefile.in (1)
161-164: LGTM! Well-structured code formatting target.The new
code-formattarget provides a clear entry point for developers to set up formatting hooks. The dependency onsubmake-generated-headersensures headers are available before formatting runs..coderabbit.yaml (1)
1-10: LGTM! Configuration is valid and aligns with PR objectives.The CodeRabbit configuration is minimal yet complete:
- Valid schema reference and language setting
- "chill" profile is appropriate for balanced feedback
base_branchescorrectly restricts automatic reviews to "master" and "IVORY_REL_5_STABLE" as stated in the PR descriptionAll settings conform to the schema provided.
contrib/ivorysql_ora/src/datatype/oratimestamptz.c (1)
1-757: LGTM! Formatting changes from pg_bsd_indent.The changes in this file are cosmetic formatting adjustments from running
pg_bsd_indent, including:
- Whitespace and blank line adjustments in header comments
- Variable declaration spacing
- Function call argument formatting
- Minor comment text refinements
No functional changes or logic alterations are present. These changes align with the PR's objective to refactor NLS code using pg_bsd_indent.
README_CN.md (1)
21-21: LGTM! Documentation updated to reflect new code formatting target.The command has been correctly updated from
make enable-git-hookstomake code-format, aligning with the new target introduced in GNUmakefile.in. The alternative bash command is preserved for users who prefer direct invocation.README.md (1)
35-35: LGTM! Documentation consistently updated across both README files.The command has been updated from
make enable-git-hookstomake code-formatwith a clear note about equivalence to the underlying script. This change:
- Aligns with the new target in GNUmakefile.in
- Maintains consistency with README_CN.md
- Provides clearer intent through improved naming
contrib/ivorysql_ora/sql/ora_datetime.sql (1)
146-146: LGTM! Comment formatting standardized.The change from triple-dash (
---) to double-dash (--) comments aligns with standard SQL commenting conventions and improves consistency.Also applies to: 279-279
contrib/ivorysql_ora/src/datatype/oradate.c (1)
1-746: LGTM! Formatting improvements only.All changes in this file are whitespace and alignment adjustments. No functional or logic changes detected.
contrib/ivorysql_ora/src/builtin_functions/datetime_datatype_functions.c (1)
1-2119: LGTM! Comprehensive formatting improvements.This file contains extensive whitespace, indentation, and alignment improvements. All changes are cosmetic with no behavioral modifications.
contrib/ivorysql_ora/src/datatype/oratimestamp.c (2)
217-218: LGTM! Error message clarity improved.The error message now correctly describes that fractional seconds beyond 6 digits will be treated as 0, which helps users understand the precision limitation.
1-1705: LGTM! Formatting improvements throughout.The remaining changes are whitespace, indentation, and alignment adjustments that improve code consistency.
contrib/ivorysql_ora/src/datatype/oratimestampltz.c (2)
129-130: LGTM! Error message clarity improved.Consistent with the change in oratimestamp.c, this message now properly explains the precision behavior.
1-738: LGTM! Formatting improvements throughout.All remaining changes are whitespace and alignment adjustments that maintain consistency with the codebase formatting standards.
src/backend/utils/misc/guc_tables.c (7)
579-579: LGTM! Formatting adjustment.The indentation change to
nls_length_semanticsinitialization is consistent with code formatting standards.
2154-2156: LGTM! Intentional GUC block repositioning.The Ivy-generated boolean GUC parameters are now inserted at this location within the
ConfigureNamesBoolarray. This repositioning is part of the NLS refactor mentioned in the PR objectives and does not affect runtime behavior.
3892-3894: LGTM! Consistent with GUC block refactoring.The integer GUC parameters are repositioned consistently with the boolean parameters reviewed earlier.
4177-4179: LGTM! Real GUC parameters repositioned.Consistent with the refactoring approach for other GUC parameter types.
4990-4992: LGTM! String GUC parameters repositioned.Continues the consistent refactoring pattern across all GUC parameter types.
5134-5135: LGTM! Minor formatting adjustment.The line break in the
gettext_noopcall foricu_validation_levelis a cosmetic formatting change.
5450-5452: LGTM! Enum GUC parameters repositioned.Completes the consistent refactoring pattern across all GUC parameter types (bool, int, real, string, enum).
src/backend/utils/misc/ivy_guc.c (3)
1-29: LGTM! Header comment formatting.The adjustments to asterisk alignment in the copyright and file header comments are consistent with code formatting standards.
33-54: LGTM! Variable declaration formatting.The spacing and indentation adjustments to global variable declarations improve code alignment without changing any types or initial values.
103-103: LGTM! Comprehensive formatting improvements.The remaining changes throughout the file are all cosmetic formatting adjustments including:
- Function signature spacing (line 103)
- Comment wrapping for better readability (lines 149-151)
- Consistent indentation in configuration structures (lines 240, 288, 299, 310, 407-409)
- Function body formatting (lines 449-493)
- Variable declaration alignment (lines 521, 540-541, 577)
- Conditional statement formatting (lines 569, 587)
All changes maintain the existing logic and values while improving code consistency. This aligns with the PR objective of using
pg_bsd_indentfor code formatting.Also applies to: 149-151, 240-240, 288-288, 299-299, 310-310, 407-409, 449-493, 521-577, 587-587
cherry-pick from #918
Summary by CodeRabbit