create_query_task triggers ":: NoMethodError : undefined method `scan' for 1:Fixnum"

mcranston18 commented 3 years ago

@joeldodge79 We have been dealing with your support team who recommended filing a bug report against the Python SDK. We are dealing with a bug specific to our project, though it may impact others. I have done my best to strip out proprietary details and just relevant information. Please let me know if you need more information; I may have to provide that to you in a non public forum.

Python Version

3.7.5

SDK Version

looker-sdk==0.1.3b20

Description

For some queries, create_query_task return a successful task. For others, it returns a generic error:

Traceback (most recent call last):
  File "app/run.py", line 33, in <module>
    run()
  File "app/run.py", line 26, in run
    query_id=query.id, result_format="json"
  File "/Users/michaelcranston/src/looker/venv/lib/python3.7/site-packages/looker_sdk/sdk/api31/methods.py", line 5439, in create_query_task
    transport_options=transport_options,
  File "/Users/michaelcranston/src/looker/venv/lib/python3.7/site-packages/looker_sdk/rtl/api_methods.py", line 184, in post
    return self._return(response, structure)
  File "/Users/michaelcranston/src/looker/venv/lib/python3.7/site-packages/looker_sdk/rtl/api_methods.py", line 88, in _return
    raise error.SDKError(response.value.decode(encoding=encoding))

We also witness this Ruby exception in the Looker dashboard:

:: NoMethodError : undefined method `scan' for 1:Fixnum

Offending query

The following query triggers the error:

query = sdk.create_query(
    body=looker_sdk.models.WriteQuery(
        fields=[
            "performance_agg.account_id",
            "performance_agg.impression",
        ],
        view="performance_agg",
        filters={
            "performance_agg.account_id_filter": "1",
        },
        model="performance_agg",
    )
)
query_task = sdk.create_query_task(
    body=looker_sdk.models.WriteCreateQueryTask(
        query_id=query.id, result_format="json"
    )
)

Notes

account_id_filter can be an integer or a string and it will have no affect
changing the account_id_filter to another account (e.g. 12) will allow the query to succeed

account_id_filter is defined as follows with actual field names replaced with foo_bar:

filter: account_id_filter {
type: number
default_value: "{{ _user_attributes['foo_bar'] }}"
required_access_grants: [foo_bar_123]
sql: {% condition %} ${foo_bar} {% endcondition %} AND 2 = 2 ;;
}

tdaelemans commented 3 years ago

Just to chime in here as the LookML developer on the project. I'm only using 2 = 2 in the SQL definition of the filter to easily find the condition in the generated query while testing.

joeldodge79 commented 3 years ago

@mcranston18 sorry, this one slipped through the cracks. Is this still an issue for you folks? when you say

changing the account_id_filter to another account (e.g. 12) will allow the query to succeed

does that mean these calls work for any other value except "1" (or 1 if I understand correctly)?

We also witness this Ruby exception in the Looker dashboard

I don't quite follow, where are you seeing that error message, in the Looker UI when loading a dashboard?

Also, is that the error message the python sdk returns (I see the traceback you posted but not the actual error message)

mcranston18 commented 3 years ago

@joeldodge79 I'm no longer on the project that was using the Looker SDK so I won't be able to provide any further context. It was still an issue when I left and I know the team was dealing with Looker support.

tdaelemans commented 3 years ago

Hi @joeldodge79.

We patched around the issue by making sure we never get into the bugged state in the first place. Effectively, we do a conditional check for the correct data type that Looker does not seem to do.

If I recall, Looker Support eventually diagnosed the cause as having to do something with a non-valid query being cached. Basically, if I recall we needed to pass Account ID filter as a string but passing it as a number (or maybe it was vice versa) would break cause Looker to break. Looker would then "cache" this bad query regardless of whether 1 or "1" was set for subsequent requests. We were developing using the SDK so we just always cast the value to a string in our code.

In our case "1" or 1 was the problem, but that was just because the cached query was using 1, it could in theory happen for any value.

Finally the Ruby error was seen in the "log" section of our instance. We would run the bad query and I would see the NoMethodError.

tdaelemans commented 3 years ago

@joeldodge79 This is what my conversation with a Looker Support Engineer eventually surfaced, if helpful. If you have access the Support Ticket ID was 355614.

After running the python sdk script against your instance, I was able to reproduce the issue, which allowed me to do some further testing and identify what I believe to be the root cause.

The create_query endpoint allows us to specify a filter value that is either a string (quoted) or an integer (unquoted), no errors are returned either by the API or by the python SDK when we do this. However, once the query is created, the filter value type cannot be changed. From our API docs here:

If you create a query that is exactly like an existing query then the existing query will be returned and no new query will be created.

So if the first time we run create_query we use an unquoted value, every subsequent call will return that same unquoted value -- even if we specify a quoted filter value. This is why the value 1 always returned a query object that was unquoted, while 12 always returned as quoted, these two query objects were originally created as unquoted and quoted respectively.

After create_query, a call to create_query_task will succeed if the filter value is quoted, and throw an application error when unquoted. I was able to reproduce this on an internal instance with a completely different model / data set. Overall the behavior is unexpected, so I have reported this to engineering. What's not quite clear to me, is if the issue is that create_query shouldn't allow us to create filter values of type int, or if the issue is that create_query_task throws an application error when presented with a filter value of type int.

ags2121 commented 3 years ago

@tdaelemans did you ever figure out how to address the caching issue? like was there a way to force the query to not be cached? I'm experiencing this same problem and wondering how to work around it...

tdaelemans commented 3 years ago

@ags2121 No, we ended up coding defensively around the issue in our application (always cast the offending field to a string so the query that is cached couldn't be broken). The previously cached queries were/are still broken, but I busted the cache by appending 1=1 to every query task after we caught this issue so technically a new query was created/stored in the Looker system. This means between the validation and the 1=1 appending all new queries are valid and avoid the bugged state.

tdaelemans commented 3 years ago

On your explore set "sql_always_where: 1=1 ;;"

On Wed, May 19, 2021 at 1:20 PM Alex Silva @.***> wrote:

do you mind sharing a code sample for how you appended 1=1 to the query?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/looker-open-source/sdk-codegen/issues/385#issuecomment-844308922, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLIKHV3WYASZGRAONZ6INTTOPXORANCNFSM4T3WU5KQ .

looker-open-source / sdk-codegen