Open
Conversation
Report line number on each ERROR/WARNING Catch errors at InputFunctionCall inside NextCopyFrom Tabs vs spaces consistency Fixed possible Segmentation fault during malformed lines processing Propagate catched SQL error code to warning IGNORE_ERRORS COPY SQL option added (only new syntax supported) Regression test updated to handle new COPY functionality Fixed parser and rebase typo
ololobus
pushed a commit
that referenced
this pull request
Oct 22, 2018
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 #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 #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, #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
ololobus
pushed a commit
that referenced
this pull request
Oct 22, 2018
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 #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 #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, #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
5fcc467 to
5c4df12
Compare
d2d0925 to
aebce07
Compare
7c01f90 to
16485ab
Compare
e8d65bf to
6556e62
Compare
8542a98 to
2603ea1
Compare
7c657da to
0e5f4f7
Compare
28db880 to
7f8e356
Compare
b32538b to
fb6f525
Compare
5d7849e to
afd25cc
Compare
2843bba to
9fcc5b8
Compare
7fb7114 to
9dd5d33
Compare
2435350 to
5a0dfc6
Compare
5a5411e to
63ab42a
Compare
2943f01 to
c87ec8b
Compare
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.
Rebased and squashed version of #3
Report line number on each ERROR/WARNING
Catch errors at InputFunctionCall inside NextCopyFrom
Tabs vs spaces consistency
Fixed possible Segmentation fault during malformed lines processing
Propagate catched SQL error code to warning
IGNORE_ERRORS COPY SQL option added (only new syntax supported)
Regression test updated to handle new COPY functionality
Fixed parser and rebase typo