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

Correct format of columns for MiqExpression #11415

Merged

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Sep 21, 2016

On some places we are using columns in bad format- for separation of columns is correct to use "-".
I need this for #11369
because there is used for parsing column Field.parse and in this case is specs were failing.

I guess that it was ok because method MiqExpression#to_ruby had correct behaviour but MiqExpression#to_sql - but it seems that it was not needed.

with "."

 MiqExpression.new({"=" => {"field" => "MiqWidget.id", "value" => 444}}).to_sql
=>
[
    [0] nil,
    [1] nil,
    [2] {
        :supported_by_sql => false
    }
]


MiqExpression.new({"=" => {"field" => "MiqWidget.id", "value" => 444}}).to_ruby
=>
"<value ref=miqwidget, type=string>/virtual/id</value> == \"444\""




with "-"


MiqExpression.new({"=" => {"field" => "MiqWidget-id", "value" => 444}}).to_sql
=>
[
    [0] "\"miq_widgets\".\"id\" = 444",
    [1] {},
    [2] {
        :supported_by_sql => true
    }
]

MiqExpression.new({"=" => {"field" => "MiqWidget-id", "value" => 444}}).to_ruby
=>
"<value ref=miqwidget, type=integer>/virtual/id</value> == 444"

cc @imtayadeway @alongoldboim

@miq-bot assign @gtanzillo

We are using "." for separation of relations
before column we are using "-".
bad:
MiqReport.id

correct:
MiqReport-id
@miq-bot
Copy link
Member

miq-bot commented Sep 21, 2016

Checked commit lpichler@fdc64a5 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
6 files checked, 1 offense detected

spec/models/miq_schedule_filter_spec.rb

@kbrock
Copy link
Member

kbrock commented Sep 21, 2016

It looks like it was working by sliding through the sql aspect, and then in ruby doing its work.

This is a bug and thanks for the fix

@kbrock kbrock added this to the Sprint 47 Ending Oct 3, 2016 milestone Sep 21, 2016
@kbrock kbrock merged commit d69fad9 into ManageIQ:master Sep 21, 2016
@kbrock
Copy link
Member

kbrock commented Sep 21, 2016

/fyi @gtanzillo @imtayadeway this is a bug and I merged it

@gtanzillo
Copy link
Member

Thanks @kbrock 👍

@lpichler lpichler deleted the use_proper_format_columns_in_miq_expression branch October 2, 2016 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants