fix: resolve all compiler warnings in extension source#246
Merged
Conversation
- Remove unused variables (build.c: metabuf/metapage/metap/snapshot) - Add missing prototype for tp_extract_terms_from_tsvector to am.h - Silence unused parameter warnings with (void) casts in dshash callbacks (posting.c, stringtable.c) and parallel build (build_parallel.c) - Remove excess initializer elements in relopt_parse_elt (handler.c) - Fix shadow variable in score.c (remove inner redeclaration of i) - Initialize min_page/min_offset to suppress maybe-uninitialized (merge.c) - Suppress -Wpacked-not-aligned for intentionally packed on-disk structs - Fix address-of-packed-member in scan.c via local + memcpy - Suppress -Wclobbered false positives from PG_TRY inlining (query.c, state.c)
- Restore snapshot variable needed for PG18's UnregisterSnapshot, suppress set-but-not-used with (void) cast on PG17 - Use designated initializers for relopt_parse_elt to handle PG17 (3 fields) vs PG18 (4 fields with isset_offset) without warnings
CI was passing CFLAGS="-Werror" to make, but pgxs overrides CFLAGS from pg_config, so -Werror was silently dropped. Warnings were never treated as errors despite the CI step suggesting they were. Switch to PG_CFLAGS="-Werror -Wno-error=unused-parameter" which pgxs correctly appends to the compiler flags. The unused-parameter exclusion is needed for warnings from Postgres headers (ilist.h, bufmgr.h) that we cannot fix. Also initialize dict_buf in segment.c to fix a maybe-uninitialized warning that -Werror now catches.
Move -Wno-clobbered and -Wno-packed-not-aligned to PG_CPPFLAGS in the Makefile instead of wrapping each site with 4-line #if/#pragma blocks. Add -Wno-unknown-warning-option so clang (used for .bc bitcode) silently ignores the GCC-specific flags. Removes 28 lines of pragma boilerplate from 3 source files.
c17304a to
04ed729
Compare
- CID 644692: Add null check after merge_sink_init_pages in tp_merge_level_segments before dereferencing sink.writer.pages - CID 644691: Same null check pattern in tp_build_parallel (original function removed but same pattern existed) - CID 644690: Zero-initialize iter.block_access in tp_segment_posting_iterator_init to avoid uninitialized buffer field when calling _free - CID 644689: Always allocate docmap output arrays in build_merged_docmap (palloc(0) is valid in PG), removing conditional NULL assignment - CID 644341: Remove dead code in tp_score_documents by making multi-term BMW path unconditional (query_term_count >= 2 is guaranteed by the early return above)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
build.c(metabuf,metapage,metap,snapshot)tp_extract_terms_from_tsvectortoam.h(removes-Wmissing-prototypesand theexterninbuild_parallel.c)(void)casts in dshash callbacks (posting.c,stringtable.c) and parallel build estimation (build_parallel.c)relopt_parse_elt(handler.c) — PG17 struct has 3 fields, not 4score.c(innerint ishadowed function-scopei)min_page/min_offsetto suppress-Wmaybe-uninitialized(merge.c)-Wpacked-not-alignedfor intentionally packed on-disk structs (segment.h) with GCC-only diagnostic pragmas-Waddress-of-packed-memberinscan.cby using localItemPointerData+memcpyinstead of taking address of packed member directly-Wclobberedfalse positives from GCC inlining functions into PG_TRY callers (query.c,state.c)After this change,
make -jproduces zero source-level warnings (only the expected pgxs Makefile recipe override notice).Test plan
make format-checkpassesmake clean && make -j$(nproc)produces no source warnings