8000 Correct sql type for replication set table by carbonin · Pull Request #2 · 2ndQuadrant/postgres · GitHub
[go: up one dir, main page]
Skip to content

Correct sql type for replication set table #2

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Correct sql type for replication set table #2

wants to merge 2 commits into from

Conversation

carbonin
Copy link

This was causing a value mismatch between the join table replication_set_table and the table replication_set.

Without the change the new regression would have failed with the following:

*** /home/ncarboni/Source/postgres/contrib/pglogical/expected/replication_set.out   2016-04-26 12:42:24.552244359 -0400
--- /home/ncarboni/Source/postgres/contrib/pglogical/results/replication_set.out    2016-04-26 12:45:17.228807573 -0400
***************
*** 93,105 ****

  -- test join table contents
  SELECT * FROM pglogical.replication_set_table ORDER BY 1,2;
!    set_id   |               set_reloid               
! ------------+----------------------------------------
!   128878480 | normalschema.test_normalschema
!   128878480 | "strange.schema-IS".test_strangeschema
!   348382733 | normalschema.test_normalschema
!  1767380104 | test_publicschema
!  3063060354 | test_publicschema
  (5 rows)

  -- should fail
--- 93,105 ----

  -- test join table contents
  SELECT * FROM pglogical.replication_set_table ORDER BY 1,2;
!    set_id    |               set_reloid               
! -------------+----------------------------------------
!  -1231906942 | test_publicschema
!    128878480 | normalschema.test_normalschema
!    128878480 | "strange.schema-IS".test_strangeschema
!    348382733 | normalschema.test_normalschema
!   1767380104 | test_publicschema
  (5 rows)

  -- should fail

======================================================================

The large hash value for the set name miq was causing signed overflow in the integer type which caused the mismatch.

carbonin added 2 commits April 26, 2016 12:42
@carbonin
Added failing regression
2eaa666 
This replication set generates a hash (oid) that causes the integer
value in replication_set_table to overflow and become negative

This can be fixed by using oid type in replication_set_table to match
the type of set_id in replication_set
@carbonin
Changed set_id in replication_set_table to oid type
a6bd2fd 
Using integer type was causing unsigned values to overflow
@carbonin
Copy link
Author

Closing as this was fixed in 2ndQuadrant/pglogical@a508512

@carbonin carbonin closed this May 13, 2016
@carbonin carbonin deleted the correct_sql_type_for_replication_set_table branch June 10, 2016 13:13
ringerc pushed a commit that referenced this pull request Mar 27, 2017
@anarazel
Blindly attempt to fix sepgsql tests #2.
d253b0f
ringerc pushed a commit that referenced this pull request Sep 11, 2018
@tglsfdc
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY.
6497a18 
refresh_by_match_merge() has some issues in the way it builds a SQL
query to construct the "diff" table:

1. It doesn't require the selected unique index(es) to be indimmediate.
2. It doesn't pay attention to the particular equality semantics enforced
by a given index, but just assumes that they must be those of the column
datatype's default btree opclass.
3. It doesn't check that the indexes are btrees.
4. It's insufficiently careful to ensure that the parser will pick the
intended operator when parsing the query.  (This would have been a
security bug before CVE-2018-1058.)
5. It's not careful about indexes on system columns.

The way to fix postgres#4 is to make use of the existing code in ri_triggers.c
for generating an arbitrary binary operator clause.  I chose to move
that to ruleutils.c, since that seems a more reasonable place to be
exporting such functionality from than ri_triggers.c.

While #1, #3, and postgres#5 are just latent given existing feature restrictions,
and #2 doesn't arise in the core system for lack of alternate opclasses
with different equality behaviors, postgres#4 seems like an issue worth
back-patching.  That's the bulk of the change anyway, so just back-patch
the whole thing to 9.4 where this code was introduced.

Discussion: https://postgr.es/m/13836.1521413227@sss.pgh.pa.us
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@carbonin
0