Skip to content

Fix PG version check for structs in separate header files #11278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sensei-hacker merged 1 commit into from
Jan 22, 2026
+17 −0
Merged

Fix PG version check for structs in separate header files #11278

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/scripts/check-pg-versions.sh
View file
Open in desktop
Copy link
Contributor

@qodo-code-review qodo-code-review bot Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-level Suggestion

The current method of parsing C code with sed and grep is fragile. A better long-term solution is to use a compiler-based tool to analyze the code's Abstract Syntax Tree (AST), which would provide a more reliable way to detect struct changes. [High-level, importance: 9]

Solution Walkthrough:

Before:

# In check-pg-versions.sh
struct_pattern="typedef struct ${struct_type%_t}_s"

# Get diff for current file
diff_output=$(git diff ...)

# Try to find struct definition using sed
struct_body_diff=$(echo "$diff_output" | sed -n "/${struct_pattern}/, /.../p")

# If not found, try companion file
if [ -z "$struct_body_diff" ]; then
    companion_diff=$(git diff ... -- "$companion_file")
    struct_body_diff=$(echo "$companion_diff" | sed -n "/${struct_pattern}/, /.../p")
fi

# Find changes in the text-based diff
struct_changes=$(echo "$struct_body_diff" | grep -E "^[-+]" | grep -v ...)

After:

# Conceptual script using a C parser (e.g., libclang)
import clang.cindex

def get_struct_signature(file_content, struct_name):
    # Parse code into an Abstract Syntax Tree (AST)
    idx = clang.cindex.Index.create()
    tu = idx.parse('tmp.c', unsaved_files=[('tmp.c', file_content)])

    # Find struct in AST and generate a signature from its members
    # (This is far more robust than text matching)
    ...
    return signature

# Get file content from base and head commits
base_content = git.show(f"{BASE_COMMIT}:{file}")
head_content = git.show(f"{HEAD_COMMIT}:{file}")

# Compare signatures
base_sig = get_struct_signature(base_content, "my_struct_t")
head_sig = get_struct_signature(head_content, "my_struct_t")

if base_sig != head_sig:
    print("Struct has changed!")

Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,23 @@ check_file_for_pg_changes() {
local struct_pattern="typedef struct ${struct_type%_t}_s"
# Isolate struct body from diff, remove comments and empty lines, then check for remaining changes
local struct_body_diff=$(echo "$diff_output" | sed -n "/${struct_pattern}/,/\}.*${struct_type};/p")

# If struct not found in current file, check corresponding .h or .c file
if [ -z "$struct_body_diff" ]; then
local companion_file=""
if [[ "$file" == *.c ]]; then
companion_file="${file%.c}.h"
elif [[ "$file" == *.h ]]; then
companion_file="${file%.h}.c"
fi

if [ -n "$companion_file" ] && git diff --name-only $BASE_COMMIT..$HEAD_COMMIT | grep -q "^${companion_file}$"; then
echo " 🔍 Struct not in $file, checking $companion_file"
local companion_diff=$(git diff $BASE_COMMIT..$HEAD_COMMIT -- "$companion_file")
struct_body_diff=$(echo "$companion_diff" | sed -n "/${struct_pattern}/,/\}.*${struct_type};/p")
fi
fi
Comment on lines +110 to +115
Copy link
Contributor

@qodo-code-review qodo-code-review bot Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: To improve performance, avoid repeated git diff calls inside the loop by getting all diffs once at the start of the script and storing them in an associative array for later lookup. [general, importance: 6]

Suggested change
if [ -n "$companion_file" ] && git diff --name-only $BASE_COMMIT..$HEAD_COMMIT | grep -q "^${companion_file}$"; then
echo " 🔍 Struct not in $file, checking $companion_file"
local companion_diff=$(git diff $BASE_COMMIT..$HEAD_COMMIT -- "$companion_file")
struct_body_diff=$(echo "$companion_diff" | sed -n "/${struct_pattern}/,/\}.*${struct_type};/p")
fi
fi
# This suggestion requires changes outside the visible diff.
# At the start of the script, you would add:
# declare -A all_diffs
# while IFS= read -r changed_file; do
# all_diffs["$changed_file"]=$(git diff $BASE_COMMIT..$HEAD_COMMIT -- "$changed_file")
# done < <(git diff --name-only $BASE_COMMIT..$HEAD_COMMIT)
# Then, the existing code block would be changed to:
if [ -n "$companion_file" ] && [ -v "all_diffs[$companion_file]" ]; then
echo " 🔍 Struct not in $file, checking $companion_file"
local companion_diff=${all_diffs[$companion_file]}
struct_body_diff=$(echo "$companion_diff" | sed -n "/${struct_pattern}/,/\}.*${struct_type};/p")
fi


local struct_changes=$(echo "$struct_body_diff" | grep -E "^[-+]" \
| grep -v -E "^[-+]\s*(typedef struct|}|//|\*)" \
| sed -E 's://.*$::' \
Expand Down