Incubator-superset: expression in table columns is converted to literal column twice

[rhp4515]

if ddtm_expr is an expression with special characters then timestamp_grain escapes the special characters already escaped

models.py 660

Steps to reproduce:

1. Have a column which has unix timestamp
2. Create a new dttm column 'date' with date_format(from_unixtime(ts), '%Y-%m-%d')
3. Run any time-series based visualization query

Problem:

1. % is escaped twice so %Y becomes %%Y and then becomes %%%%Y

[rhp4515]

@mistercrunch

[tan31989]

I tried looking into this issue and found that there is this sqlalchemy method literal_column which uses a replace text method and is reported as a bug in sqlalchemy project: https://bitbucket.org/zzzeek/sqlalchemy/issues/3740/clean-up-doubling-of-percent-signs-base-on

Will try and see the update of whether it can be directly used with help of sqlalchemy's text method.

[tan31989]

After interacting with the Sqlalchemy issue support, #3737 - Bitbucket,

I have implemented and tested the solution to escape the double percentage getting displayed for literal_column. This solution is basically a decorated method that takes care of removing back the added percentages for type of DateTime in ColumnClause.

[yejianye]

Met the same problem, hope the PR could be merged soon.

[yejianye]

Also notice 'week' and 'month' time grain is broken for SQLite because of this issue as well. Both have percent sign in its sql expression.

            'sqlite': (
                Grain('Time Column', _('Time Column'), '{col}'),
                Grain('day', _('day'), 'DATE({col})'),
                Grain("week", _('week'),
                      "DATE({col}, -strftime('%w', {col}) || ' days')"),
                Grain("month", _('month'),
                      "DATE({col}, -strftime('%d', {col}) || ' days')"),
            ),

[rhunwicks]

As @yejianye pointed out this issue also breaks 'week' and 'month' time grains on SQLite but the fix that was applied only applies to column expressions, so I think that time grains are still broken for SQLite.

[stonecharioteer]

@rhunwicks Time grains are still broken! Is there another bug report related to this?

[rhunwicks]

@vinay87 I'm not aware of another bug report - I think this one should be reopened.

[mistercrunch]

There's a bunch of tangled issues being discussed here, can someone open a new issue that explains exactly what is going on? I'm pretty sure the double % is not an issue anymore.

[rhunwicks]

@mistercrunch when you run a query against a SQLite database that has a time grain of week or month, you pass a query_obj something like:

        {
            'groupby': [],
            'metrics': ['sum__value'],
            'granularity': 'received',
            'from_dttm': datetime.datetime(2001, 1, 1),
            'to_dttm': datetime.datetime(2001, 12, 31),
            'filter': [],
            'is_timeseries': True,
            'timeseries_limit': 50,
            'timeseries_limit_metric': None,
            'row_limit': 5000,
            'extras': {
                'time_grain_sqla': 'month',
            },
            'order_desc': True,
            }

The time grain code for SQLite uses strftime to truncate the date, but the %d is doubled up to %%d and consequently the strftime doesn't work, and so the date doesn't get truncated.

The SQL that gets generated by that query_obj is

SELECT
DATE(received, -strftime('%%d', received) || ' days', '+1 day') AS __timestamp,
SUM(value) AS sum__value 
FROM test_datasource 
WHERE received >= '2001-01-01 00:00:00.000000' AND received <= '2001-12-31 00:00:00.000000' GROUP BY DATE(received, -strftime('%%d', received) || ' days', '+1 day')
ORDER BY sum__value DESC

[betodealmeida]

@mistercrunch can confirm this is still a problem, I hit it today. Will take a look.

[betodealmeida]

This should be fixed in SQLAlchemy 1.2: http://docs.sqlalchemy.org/en/latest/changelog/migration_12.html#percent-signs-in-literal-column-now-conditionally-escaped

[betodealmeida]

I tested with 1.2.1 and can confirm it works, here's the generated query without the %%: