fix: multiple bug fixes and dead code cleanup#75
Open
hobostay wants to merge 2 commits intoNikolayS:mainfrom
Open
fix: multiple bug fixes and dead code cleanup#75hobostay wants to merge 2 commits intoNikolayS:mainfrom
hobostay wants to merge 2 commits intoNikolayS:mainfrom
Conversation
1. Go consumer: add Nack() method and use per-message nack instead of skipping the entire batch on handler failure. Log warning for unregistered event types instead of silently dropping messages. 2. maint(): use quote_ident() for VACUUM target to prevent potential SQL injection via dynamic SQL construction. 3. insert_event_raw(): add explicit queue existence check with clear error message instead of NULL pointer crash on missing queue. 4. dlq_replay_all(): wrap per-event replay in exception block so a single failure (e.g. concurrent queue drop) doesn't abort the entire replay operation. 5. upgrade_schema(): fix inverted role membership — pgque_admin was created as a member of reader/writer instead of the reverse. Also return cnt instead of hardcoded 0. 6. tune_storage(): remove dead PG 8.3 code path (pg_autovacuum catalog) and unnecessary version check — PgQue requires PG14+. 7. maint_tables_to_vacuum(): remove references to non-existent schemas (pgq_ext, pgq_node, londiste, txid) left over from PgQ fork. 8. Fix typos in comments: consubers→consumers, poin→point, oldes→oldest, wuith→with, accitentally→accidentally, Quete→Quote. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Owner
|
@hobostay thank you It would be great to split this PR into pieces - I see two different classes things here:
|
Remove Class 1 changes (typos, PG 8.3 dead code removal, non-existent schema references) as requested by maintainer. These should be pushed upstream to pgq/pgq instead. Only bug fixes remain in this PR. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Author
|
@NikolayS Thank you for the feedback! I've updated the PR to remove all typo fixes and dead code cleanup (PG 8.3 This PR now only contains the substantive bug fixes:
I'll look into submitting the typo fixes upstream to pgq/pgq separately. |
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
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
Nack()method; use per-message nack for handler failures instead of skipping the entire batch; log warning for unregistered event types instead of silently dropping messagesquote_ident()inmaint()for VACUUM target; add explicit queue existence check ininsert_event_raw()with clear error messagedlq_replay_all()now wraps per-event replay in exception blocks so a single failure doesn't abort the entire operationupgrade_schema()had inverted role membership —pgque_adminwas created as member of reader/writer instead of the reverse; also fixed return value (was hardcoded0, now returns actual count)pg_autovacuumcode path (PgQue requires PG14+); remove references to non-existent schemas (pgq_ext,pgq_node,londiste,txid) frommaint_tables_to_vacuum()consubers→consumers,poin→point,oldes→oldest,wuith→with,accitentally→accidentally,Quete→QuoteTest plan
go vetpgque.sqlupgrade_schema()role hierarchy is correct (pgque_reader → pgque_writer → pgque_admin)dlq_replay_all()with a concurrent queue drop to verify exception handlinginsert_event_raw()raises clear error for non-existent queue🤖 Generated with Claude Code