Skip to content
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.

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

Fix updating of indexes containing escaped characters #156

Merged

Conversation

antonivanopoulos
Copy link
Contributor

There are a number of different mechanisms with String#sub that pertain to the matching of backreferences and capture groups when using either the String or Hash type args for the replacement arg.

If replacement is a String that looks like a pattern's capture group but is actually not a pattern capture group e.g. "'", then it will have to be preceded by two backslashes like so "\'".

When an index contains unescaped characters, those are being incorrectly interpreted as capture groups. Using the block version of #sub avoids this.

For example, here's the behaviour we see with an index containing special characters that need escaping.

Existing index:

# Indexes
# 
# index_xyz   (regexp_replace((some_column)::text, '[[:space:]]|\+61|^[0]'::text, ''::text, 'g'::text))

Updated index:

# Indexes
# 
# index_xyz   (regexp_replace((some_column)::text, '[[:space:]]|61|^[0]'::text, ''::text, 'g'::text))

This becomes a problem given that the issue is in the Updater itself; we get stuck in a loop where annotation changes are always being detected, so a CI step with --frozen will always fail.

…th new content containing escapted characters
There are a number of different mechanisms with String#sub that pertain
to the matching of backreferences and capture groups when using either
the String or Hash type args for the replacement arg.

When an index contains unescaped characters, those are being incorrectly
interpreted as capture groups.

>If replacement is a String that looks like a pattern's capture group
but is actually not a pattern capture group e.g. "\'", then it will have
to be preceded by two backslashes like so "\\'".

Using the block version of #sub avoids this.
@drwl
Copy link
Owner

drwl commented Oct 14, 2024

@antonivanopoulos thanks for finding this bug and submitting this fix. A really interesting difference in behavior between the two modes. I was able to reproduce it myself, for anyone future viewer that is interested:

file_content = <<~FILE
  # An Annotation
  class User < ApplicationRecord
  end
FILE

current_annotations = <<~CURR
  # An Annotation
CURR

new_annotations = <<~ANNOTATIONS
  #  index_rails_02e851e3b8  (another_column) WHERE value IS LIKE '\\\\%'
  #
ANNOTATIONS

puts file_content.sub(current_annotations, new_annotations)
# =>
# #  index_rails_02e851e3b8  (another_column) WHERE value IS LIKE '\%'
# #
# class User < ApplicationRecord
# end

puts file_content.sub(current_annotations) { new_annotations }
# =>
# #  index_rails_02e851e3b8  (another_column) WHERE value IS LIKE '\\%'
# #
# class User < ApplicationRecord
# end
# => nil

@drwl drwl merged commit 963bbc7 into drwl:main Oct 14, 2024
22 checks passed
@drwl
Copy link
Owner

drwl commented Oct 14, 2024

Also apologies for delay in review. I have more time now to dedicate back to maintaining this gem.

@antonivanopoulos
Copy link
Contributor Author

No problem at all, thanks for taking a look 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants