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

Dealing with SET Edge cases #70

Closed
dvodvo opened this issue Dec 9, 2014 · 3 comments · Fixed by #80
Closed

Dealing with SET Edge cases #70

dvodvo opened this issue Dec 9, 2014 · 3 comments · Fixed by #80

Comments

@dvodvo
Copy link

dvodvo commented Dec 9, 2014

As README states: The only edge case is SET operations which are sent to all connections. Devise controller is using SET and thus leading to an error which one would like to avoid:
PG::ReadOnlySqlTransaction: ERROR: cannot execute UPDATE in a read-only transaction : UPDATE "users" SET "last_sign_in_at" = '2014-12-09 19:58:58.290982', [...]
notwithstanding the creation of an sql_proxy.rb initializer that should only handle select on the slave, as follows:

class MakaraSqlProxy < ::Makara::Proxy
  def connection_for(config)
    ::Sql::Client.new(config)
  end

  hijack_method :select, :ping
  send_to_all :connect, :reconnect, :disconnect, :clear_cache

  def needs_master?(method_name, args)
    return false if args.empty?
    sql = args.first
    sql !~ /^select/i
  end

end
righi added a commit to righi/makara that referenced this issue Apr 26, 2015
righi added a commit to righi/makara that referenced this issue Apr 26, 2015
righi added a commit to righi/makara that referenced this issue Apr 26, 2015
@righi
Copy link
Contributor

righi commented Apr 26, 2015

I ran into this problem a few times recently with INSERTS and UPDATES that contained a line feed followed by the word "set".

For example:

sql = %Q{
  UPDATE
    dogs
  SET
    max_treats = 10
  WHERE
    max_treats IS NULL
}
proxy.execute(sql) # Fails with "PG::ReadOnlySqlTransaction: ERROR: cannot execute INSERT in a read-only transaction

As a temporary workaround I combined my SQL into a single line, but then I hit the same problem again, this time on an UPDATE with user-defined input:

athlete.update(bio: params[:bio])

# params[:bio] comes from a textarea and will fail when it contains
# the word "set" at the beginning of a line other than the first.
# For example:
# "On January 18 2014 I
# set a Guinness World Record
# completing 50 Aztec push-ups
# in one minute"

The problem is with the SQL_ALL_MATCHERS regex which is too aggressive in it's match of the word "set".

I created a failing spec that proves the bug and will take a crack at improving the regex.

@mnelson
Copy link
Contributor

mnelson commented Apr 26, 2015

I can take a look some time this week. Any chance it's as simple as swapping^ to /A?

righi added a commit to righi/makara that referenced this issue Apr 26, 2015
@righi
Copy link
Contributor

righi commented Apr 26, 2015

@mnelson Thanks! That's exactly what I did.

This does introduce the possibility that SET statements don't get passed to all connections when they should. For example if somebody is using instrumentation middleware that puts comments at the beginning of statements:

proxy.execute("/*application:myapp,controller:foo,action:bar*/\nSET @@some_variable")

This probably isn't common enough to warrant a more precise regex at the expense of speed, but wanted to point it out.

favdev111 pushed a commit to favdev111/makara that referenced this issue Sep 1, 2021
favdev111 pushed a commit to favdev111/makara that referenced this issue Sep 1, 2021
favdev111 pushed a commit to favdev111/makara that referenced this issue Sep 1, 2021
favdev111 pushed a commit to favdev111/makara that referenced this issue Sep 1, 2021
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 a pull request may close this issue.

3 participants