add display ivorysql version in psql welcome banner and select(). #939
add display ivorysql version in psql welcome banner and select(). #939gaoxueyu merged 3 commits intoIvorySQL:IVORY_REL_5_STABLEfrom
Conversation
WalkthroughAdds IvorySQL version metadata to the build and runtime version strings by introducing PACKAGE_IVORYSQL_VERSION ('5.0'), exporting it in the generated config header, and embedding it into PG_VERSION_STR so runtime banners and version outputs include "(IvorySQL $PACKAGE_IVORYSQL_VERSION)". Changes
Sequence Diagram(s)(omitted — changes are metadata/string additions and minor formatting; no control-flow interaction worth diagramming) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 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: 3
🧹 Nitpick comments (2)
meson.build (1)
158-161: Good: meson exports PACKAGE_IVORYSQL_VERSION to pg_config.h.This ensures downstream C code can read the IvorySQL version in Meson builds. Consider adding a Meson option (e.g., -Divorysql_version=...) to avoid hardcoding '5.0' in the file, but not required for this PR.
src/bin/psql/command.c (1)
5842-5857: Remove unnecessary parentheses for consistencyLines 5843, 5845, and 5847 have extra parentheses around the
pstrdup()calls in return statements:
return (pstrdup("full"));return (pstrdup("column"));return (pstrdup("page"));However, line 5855 returns without the extra parentheses:
return pstrdup(wbuff);For consistency, remove the unnecessary parentheses from lines 5843, 5845, and 5847. In C, return statements don't require parentheses around the return value.
Apply this diff:
if (popt->topt.expanded_header_width_type == PRINT_XHEADER_FULL) - return (pstrdup("full")); + return pstrdup("full"); else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_COLUMN) - return (pstrdup("column")); + return pstrdup("column"); else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_PAGE) - return (pstrdup("page")); + return pstrdup("page");
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
configure(1 hunks)configure.ac(1 hunks)meson.build(2 hunks)src/bin/psql/command.c(7 hunks)src/include/pg_config.h.in(1 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
- configure
🧰 Additional context used
🧬 Code graph analysis (1)
src/bin/psql/command.c (1)
src/backend/utils/mmgr/mcxt.c (1)
pstrdup(1702-1706)
⏰ 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: pg_regression (ubuntu-latest)
- GitHub Check: oracle_pg_regression (ubuntu-latest)
- GitHub Check: meson_build (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: contrib_regression
- GitHub Check: oracle_regression (ubuntu-latest)
🔇 Additional comments (7)
configure.ac (1)
2556-2557: LGTM: IvorySQL version included in PG_VERSION_STR.Matches the desired "PostgreSQL 18.0 (IvorySQL 5.0) ..." form used in examples.
src/bin/psql/command.c (6)
45-45: LGTM: Minor formatting adjustmentThe formatting change to the include statement is fine.
314-315: LGTM: Consistent DB mode handlingThe DB_ORACLE mode handling is consistent with the pattern established earlier in the function (lines 254-258).
1062-1062: LGTM: Formatting adjustmentThe blank line addition improves readability by separating the compatibility mode retrieval from subsequent logic.
2632-2632: LGTM: Formatting improvementThe blank line addition is a minor formatting improvement.
5291-5298: LGTM: Parameter handling logicThe xheader_width parameter handling is consistent with the patterns used elsewhere in the do_pset function.
4525-4525: PACKAGE_IVORYSQL_VERSION is properly defined in the build systemVerification confirms that
PACKAGE_IVORYSQL_VERSIONis correctly configured in both Meson (meson.build:158) and Autoconf (configure.ac,configurescript). The#undefinpg_config.h.inis a template placeholder that gets properly replaced during the build process.The banner format change at line 4525 correctly implements the PR objective with proper macro definition available at compile time.
| ["PostgreSQL $PG_VERSION (IvorySQL $PACKAGE_IVORYSQL_VERSION) on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"], | ||
| [A string containing the version number, platform, and C compiler]) |
There was a problem hiding this comment.
🧩 Analysis chain
Define PACKAGE_IVORYSQL_VERSION in Autoconf header to match Meson.
To ensure src/include/pg_config.h defines PACKAGE_IVORYSQL_VERSION (not just leaves it #undef), add an option and define:
@@
AC_DEFINE_UNQUOTED(PG_VERSION, "$PG_VERSION", [PostgreSQL version as a string])
+dnl IvorySQL version string (configurable; default 5.0)
+AC_ARG_WITH([ivorysql-version],
+ [AS_HELP_STRING([--with-ivorysql-version=VER], [set IvorySQL version string [5.0]])],
+ [PACKAGE_IVORYSQL_VERSION="$withval"],
+ [PACKAGE_IVORYSQL_VERSION="5.0"])
+AC_DEFINE_UNQUOTED([PACKAGE_IVORYSQL_VERSION], ["$PACKAGE_IVORYSQL_VERSION"], [IvorySQL version as a string])
+AC_SUBST([PACKAGE_IVORYSQL_VERSION])
+
@@
AC_DEFINE_UNQUOTED(PG_VERSION_STR,
- ["PostgreSQL $PG_VERSION (IvorySQL $PACKAGE_IVORYSQL_VERSION) on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"],
+ ["PostgreSQL $PG_VERSION (IvorySQL $PACKAGE_IVORYSQL_VERSION) on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"],
[A string containing the version number, platform, and C compiler])This keeps both build systems in lockstep and lets packagers override the IvorySQL version via configure.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and inspect configure.ac around the mentioned lines and search for PACKAGE_IVORYSQL_VERSION usage
fd -t f "configure\.ac" -x cat {} | head -2600 | tail -100
Length of output: 4117
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of PACKAGE_IVORYSQL_VERSION in configure.ac
rg -n "PACKAGE_IVORYSQL_VERSION" --type autoconf -C 5Length of output: 32
🏁 Script executed:
#!/bin/bash
# Check if pg_config.h.in exists and contains PACKAGE_IVORYSQL_VERSION
fd -t f "pg_config\.h\.in" -x rg -n "PACKAGE_IVORYSQL_VERSION" {} -C 3Length of output: 209
🏁 Script executed:
#!/bin/bash
# Search for AC_DEFINE_UNQUOTED calls to understand current pattern
rg -n "AC_DEFINE_UNQUOTED.*VERSION" --type autoconf -C 2Length of output: 32
🏁 Script executed:
#!/bin/bash
# Check for Meson build configuration related to IvorySQL version
fd -t f "meson.build" -x rg -n "IVORYSQL\|ivorysql" {} -C 3Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search configure.ac for PACKAGE_IVORYSQL_VERSION without type restriction
rg "PACKAGE_IVORYSQL_VERSION" -C 5 -- configure.ac
Length of output: 500
🏁 Script executed:
#!/bin/bash
# Search for AC_DEFINE_UNQUOTED with VERSION in configure.ac
rg "AC_DEFINE_UNQUOTED.*VERSION" -C 3 -- configure.acLength of output: 1354
🏁 Script executed:
#!/bin/bash
# Look for where PACKAGE_IVORYSQL_VERSION might be initially set/assigned
rg "PACKAGE_IVORYSQL_VERSION\s*=" -C 3 -- configure.acLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search all meson files for IvorySQL version handling
find . -name "meson.build" -o -name "meson_options.txt" 2>/dev/null | head -20Length of output: 638
🏁 Script executed:
#!/bin/bash
# Check if PACKAGE_IVORYSQL_VERSION appears in any AC_SUBST or AC_ARG_WITH
rg "AC_ARG_WITH|AC_SUBST.*IVORYSQL" -C 3 -- configure.acLength of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Check root meson.build for IVORYSQL version handling
rg "IVORYSQL|ivorysql|VERSION" ./meson.build -C 3 2>/dev/null | head -50
Length of output: 1553
🏁 Script executed:
#!/bin/bash
# Check config/meson.build for version definitions
rg "IVORYSQL|ivorysql|PACKAGE_VERSION" ./config/meson.build -C 3 2>/dev/null | head -50Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for version defines or conf settings in meson files
rg "conf\.set.*IVORYSQL|conf\.set.*VERSION" . -g "meson.build" -C 3 | head -50Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's any VERSION variable or option defined in Meson
fd -t f "meson.build" -x grep -l "version\|VERSION" {} \; | head -5Length of output: 160
Add AC_DEFINE_UNQUOTED and AC_ARG_WITH for PACKAGE_IVORYSQL_VERSION in configure.ac to match Meson.
Verification confirms Meson (./meson.build) correctly defines ivorysql_version = '5.0' and sets cdata.set_quoted('PACKAGE_IVORYSQL_VERSION', ivorysql_version), but Autoconf never defines this variable. Currently, configure.ac references $PACKAGE_IVORYSQL_VERSION in AC_DEFINE_UNQUOTED(PG_VERSION_STR, ...) without defining it, causing pg_config.h to leave PACKAGE_IVORYSQL_VERSION undefined (#undef). The suggested patch aligns the Autoconf build with Meson and enables packagers to customize the IvorySQL version via --with-ivorysql-version.
@@
AC_DEFINE_UNQUOTED(PG_VERSION, "$PG_VERSION", [PostgreSQL version as a string])
+dnl IvorySQL version string (configurable; default 5.0)
+AC_ARG_WITH([ivorysql-version],
+ [AS_HELP_STRING([--with-ivorysql-version=VER], [set IvorySQL version string [5.0]])],
+ [PACKAGE_IVORYSQL_VERSION="$withval"],
+ [PACKAGE_IVORYSQL_VERSION="5.0"])
+AC_DEFINE_UNQUOTED([PACKAGE_IVORYSQL_VERSION], ["$PACKAGE_IVORYSQL_VERSION"], [IvorySQL version as a string])
+AC_SUBST([PACKAGE_IVORYSQL_VERSION])
+
@@
AC_DEFINE_UNQUOTED(PG_VERSION_STR,
["PostgreSQL $PG_VERSION (IvorySQL $PACKAGE_IVORYSQL_VERSION) on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"],
[A string containing the version number, platform, and C compiler])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ["PostgreSQL $PG_VERSION (IvorySQL $PACKAGE_IVORYSQL_VERSION) on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"], | |
| [A string containing the version number, platform, and C compiler]) | |
| dnl IvorySQL version string (configurable; default 5.0) | |
| AC_ARG_WITH([ivorysql-version], | |
| [AS_HELP_STRING([--with-ivorysql-version=VER], [set IvorySQL version string [5.0]])], | |
| [PACKAGE_IVORYSQL_VERSION="$withval"], | |
| [PACKAGE_IVORYSQL_VERSION="5.0"]) | |
| AC_DEFINE_UNQUOTED([PACKAGE_IVORYSQL_VERSION], ["$PACKAGE_IVORYSQL_VERSION"], [IvorySQL version as a string]) | |
| AC_SUBST([PACKAGE_IVORYSQL_VERSION]) | |
| AC_DEFINE_UNQUOTED(PG_VERSION_STR, | |
| ["PostgreSQL $PG_VERSION (IvorySQL $PACKAGE_IVORYSQL_VERSION) on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"], | |
| [A string containing the version number, platform, and C compiler]) |
🤖 Prompt for AI Agents
In configure.ac around lines 2556-2557, configure.ac references
$PACKAGE_IVORYSQL_VERSION in AC_DEFINE_UNQUOTED(PG_VERSION_STR, ...) but never
defines it; add an AC_ARG_WITH that accepts --with-ivorysql-version (setting
PACKAGE_IVORYSQL_VERSION to the provided value, defaulting to the Meson value
such as "5.0" if none given) and then call
AC_DEFINE_UNQUOTED(PACKAGE_IVORYSQL_VERSION, "$PACKAGE_IVORYSQL_VERSION",
[IvorySQL version string]) before the PG_VERSION_STR/AC_DEFINE_UNQUOTED usage so
pg_config.h gets a defined PACKAGE_IVORYSQL_VERSION and packagers can override
via --with-ivorysql-version.
| cdata.set_quoted('PG_VERSION_STR', | ||
| 'PostgreSQL (IvorySQL @6@) @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format( | ||
| pg_version, host_machine.cpu_family(), host_system, | ||
| cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, cdata.get('PACKAGE_IVORYSQL_VERSION') | ||
| cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, ivorysql_version | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Make PG_VERSION_STR ordering match Autoconf and example output.
Meson currently emits "PostgreSQL (IvorySQL 5.0) 18.0 ...", while Autoconf emits "PostgreSQL 18.0 (IvorySQL 5.0) ...". Align with Autoconf and the PR examples for consistency across build systems.
Apply:
- cdata.set_quoted('PG_VERSION_STR',
- 'PostgreSQL (IvorySQL @6@) @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format(
+ cdata.set_quoted('PG_VERSION_STR',
+ 'PostgreSQL @0@ (IvorySQL @6@) on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format(
pg_version, host_machine.cpu_family(), host_system,
cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, ivorysql_version
)
)
---
🏁 Script executed:
```shell
#!/bin/bash
# Locate and inspect meson.build around lines 2982-2987
if [ -f meson.build ]; then
echo "=== meson.build lines 2980-2990 ==="
sed -n '2980,2990p' meson.build
else
echo "meson.build not found in root; searching..."
find . -name meson.build -type f | head -5
fi
Length of output: 437
🏁 Script executed:
#!/bin/bash
# Search for Autoconf equivalents (configure.in, configure.ac, etc.)
echo "=== Looking for Autoconf files ==="
find . -name 'configure.*' -o -name 'config.in' -o -name 'config.ac' | head -10Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for PG_VERSION_STR definitions in Autoconf files
echo "=== Searching for PG_VERSION_STR in Autoconf context ==="
rg -n 'PG_VERSION_STR' --type=in --type=ac -C5 2>/dev/null | head -50Length of output: 56
🏁 Script executed:
#!/bin/bash
# Look for references to version ordering in comments or documentation
echo "=== Searching for version string format context ==="
rg -n 'PostgreSQL.*IvorySQL|IvorySQL.*PostgreSQL' -C3 2>/dev/null | head -40Length of output: 2668
Align PG_VERSION_STR format order between Meson and Autoconf.
Meson currently outputs "PostgreSQL (IvorySQL @6@) @0@ on...", while Autoconf outputs "PostgreSQL @0@ (IvorySQL @6@) on...". Swap the positions of @0@ (pg_version) and the IvorySQL version in the format string to match Autoconf.
- cdata.set_quoted('PG_VERSION_STR',
- 'PostgreSQL (IvorySQL @6@) @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format(
+ cdata.set_quoted('PG_VERSION_STR',
+ 'PostgreSQL @0@ (IvorySQL @6@) on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format(
pg_version, host_machine.cpu_family(), host_system,
cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, ivorysql_version
)
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cdata.set_quoted('PG_VERSION_STR', | |
| 'PostgreSQL (IvorySQL @6@) @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format( | |
| pg_version, host_machine.cpu_family(), host_system, | |
| cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, cdata.get('PACKAGE_IVORYSQL_VERSION') | |
| cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, ivorysql_version | |
| ) | |
| ) | |
| cdata.set_quoted('PG_VERSION_STR', | |
| 'PostgreSQL @0@ (IvorySQL @6@) on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format( | |
| pg_version, host_machine.cpu_family(), host_system, | |
| cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, ivorysql_version | |
| ) | |
| ) |
🤖 Prompt for AI Agents
meson.build around lines 2982 to 2987: the PG_VERSION_STR format places the
pg_version before the IvorySQL tag, which differs from Autoconf's "PostgreSQL
@0@ (IvorySQL @6@) on..." ordering; swap the positions of the pg_version and the
IvorySQL version in the format string so pg_version appears immediately after
"PostgreSQL" and the IvorySQL tag appears in parentheses, keeping the rest of
the placeholders and arguments unchanged.
| /*IvorySQL version as a string*/ | ||
| #undef PACKAGE_IVORYSQL_VERSION | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Header placeholder added; ensure Autoconf defines it.
The #undef exposes PACKAGE_IVORYSQL_VERSION, but Autoconf must define it for non‑Meson builds. Verify configure.ac (or generated configure) sets and AC_DEFINEs PACKAGE_IVORYSQL_VERSION, else clients will see it undefined.
Run:
🏁 Script executed:
#!/bin/bash
# Verify Autoconf defines PACKAGE_IVORYSQL_VERSION
rg -nP 'AC_DEFINE(_UNQUOTED)?\s*\(\s*PACKAGE_IVORYSQL_VERSION' configure.ac || true
rg -nP '\bPACKAGE_IVORYSQL_VERSION\b' configure.ac || trueLength of output: 159
Need to verify if PACKAGE_IVORYSQL_VERSION is defined via AC_SUBST or other mechanisms.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check for AC_SUBST assignment and surrounding context
rg -nP -B5 -A5 'PACKAGE_IVORYSQL_VERSION' configure.ac
Length of output: 555
Need to find where PACKAGE_IVORYSQL_VERSION is assigned.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for where PACKAGE_IVORYSQL_VERSION is assigned or AC_SUBSTed
rg -nP 'PACKAGE_IVORYSQL_VERSION\s*=' configure.ac
rg -nP 'AC_SUBST\s*\(\s*PACKAGE_IVORYSQL_VERSION' configure.ac
Length of output: 0
The variable appears undefined. Let me search more broadly for IvorySQL context and verify the pg_config.h.in file:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search broadly for IVORYSQL references
rg -nP 'IVORYSQL' configure.ac | head -20
# Check pg_config.h.in around the specified lines
echo "=== pg_config.h.in lines 630-640 ==="
sed -n '630,640p' src/include/pg_config.h.in
Length of output: 504
PACKAGE_IVORYSQL_VERSION is undefined in configure.ac; AC_DEFINE is missing.
Verification confirms the review concern is valid. PACKAGE_IVORYSQL_VERSION is used in configure.ac line 2556 within AC_DEFINE_UNQUOTED(PG_VERSION_STR, ...) but is never initialized or defined. The #undef PACKAGE_IVORYSQL_VERSION in pg_config.h.in will remain unsubstituted, and the shell expansion will produce an empty version string.
Required fix: Add AC_DEFINE_UNQUOTED(PACKAGE_IVORYSQL_VERSION, [...]) or initialize the variable in configure.ac before its use at line 2556. Without this, non-Meson builds will have an incomplete IvorySQL version string.
🤖 Prompt for AI Agents
In src/include/pg_config.h.in around lines 633-635, PACKAGE_IVORYSQL_VERSION is
left undefined so the generated header contains an unsubstituted "#undef
PACKAGE_IVORYSQL_VERSION" and configure.ac later references it, producing an
empty version string; fix by adding an
AC_DEFINE_UNQUOTED(PACKAGE_IVORYSQL_VERSION, <version-value>) (or otherwise
initialize PACKAGE_IVORYSQL_VERSION) in configure.ac before line 2556 so the
configure run will substitute the macro into pg_config.h.in and embed a correct
IvorySQL version in non-Meson builds.
There was a problem hiding this comment.
configure is usually generated by autoconf tool. Unsure if autoconf tool needs to be run each time the configure.ac is changed.
There was a problem hiding this comment.
it‘s reasonable to be concerned about that after autoconf executed, the configure file will be covered.
fix it now.
Summary by CodeRabbit
New Features
Chores