Revert "Make select_star work with SQL Lab views (#8598)" #8930

graceguo-supercat · 2020-01-06T19:49:07Z

This reverts commit 964e6db

SUMMARY

#8598 is a good feature, but in airbnb it cause some unknown issue for complicated queries. Chart returns 500 error and we got follow error message:


Traceback(most recent call last):
File "/usr/local/lib/python3.6/dist-packages/gevent/baseserver.py", line 26, in _handle_and_close_when_done
File "/usr/local/lib/python3.6/dist-packages/gunicorn/workers/ggevent.py", line 139, in handle
File "/usr/local/lib/python3.6/dist-packages/gunicorn/workers/base_async.py", line 55, in handle
File "/usr/local/lib/python3.6/dist-packages/gunicorn/workers/ggevent.py", line 143, in handle_request
File "/usr/local/lib/python3.6/dist-packages/gunicorn/workers/base_async.py", line 106, in handle_request
File "/usr/local/lib/python3.6/dist-packages/newrelic/api/wsgi_application.py", line 665, in _nr_wsgi_application_wrapper_
File "/usr/local/lib/python3.6/dist-packages/newrelic/api/wsgi_application.py", line 193, in __init__
File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 2463, in __call__
File "/usr/local/lib/python3.6/dist-packages/werkzeug/middleware/proxy_fix.py", line 228, in __call__
File "/usr/local/lib/python3.6/dist-packages/newrelic/api/wsgi_application.py", line 555, in _nr_wsgi_application_wrapper_
File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 2446, in wsgi_app
File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1949, in full_dispatch_request
File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1935, in dispatch_request
File "/usr/local/lib/python3.6/dist-packages/newrelic/hooks/framework_flask.py", line 45, in _nr_wrapper_handler_
File "/usr/local/lib/python3.6/dist-packages/flask_appbuilder/security/decorators.py", line 123, in wraps
File "/usr/local/lib/python3.6/dist-packages/superset/views/core.py", line 2231, in dashboard
File "/usr/local/lib/python3.6/dist-packages/superset/views/core.py", line 2231, in <dictcomp>
File "/usr/local/lib/python3.6/dist-packages/superset/connectors/sqla/models.py", line 549, in data
File "/usr/local/lib/python3.6/dist-packages/superset/connectors/base/models.py", line 209, in data
File "/usr/local/lib/python3.6/dist-packages/superset/connectors/sqla/models.py", line 537, in select_star
File "/usr/local/lib/python3.6/dist-packages/superset/models/core.py", line 1045, in select_star
File "/usr/local/lib/python3.6/dist-packages/superset/db_engine_specs/presto.py", line 442, in select_star
File "/usr/local/lib/python3.6/dist-packages/superset/db_engine_specs/base.py", line 669, in select_star
File "/usr/local/lib/python3.6/dist-packages/superset/models/core.py", line 1013, in compile_sqla_query
File "<string>", line 1, in <lambda>
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/elements.py", line 462, in compile
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/elements.py", line 468, in _compiler
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 562, in __init__
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 319, in __init__
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 350, in process
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/visitors.py", line 91, in _compiler_dispatch
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 2121, in visit_select
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 2213, in _compose_select_body
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 2213, in <listcomp>
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/visitors.py", line 91, in _compiler_dispatch
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 912, in visit_textclause
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 901, in do_bindparam
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 350, in process
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/visitors.py", line 91, in _compiler_dispatch
File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/sql/compiler.py", line 1476, in visit_bindparam

Because the error message is very general, i am not sure which dependency module cause this issue. We have to revert the whole PR to make master branch stable.

TEST PLAN

Manual test.

ADDITIONAL INFORMATION

Has associated issue: Make select_star work with SQL Lab views #8598

REVIEWERS

@etr2460 @betodealmeida @mistercrunch

This reverts commit 964e6db

etr2460

lgtm, thanks for keeping master healthy!

codecov-io · 2020-01-06T20:00:13Z

Codecov Report

Merging #8930 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8930   +/-   ##
=======================================
  Coverage   58.97%   58.97%           
=======================================
  Files         359      359           
  Lines       11333    11333           
  Branches     2787     2787           
=======================================
  Hits         6684     6684           
  Misses       4471     4471           
  Partials      178      178

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2726f21...0837d74. Read the comment docs.

mistercrunch · 2020-01-06T21:39:02Z

FYI I checked and it appears that #8598 is not in 0.35.*, so this revert doesn't need to be tagged with v0.35

betodealmeida · 2020-01-07T18:42:49Z

@graceguo-supercat @etr2460 can you add a unit test capturing this regression?

#8598 was a bug fix (not a feature as @graceguo-supercat mentioned), and now we're back in a worse state IMHO. If you add a unit test that captures how it broke your query I can try to work on it again.

DiggidyDave · 2020-01-10T19:43:20Z

What's the status of this? Without a clear repro/testcase what's the path to getting the original change re-merged? @graceguo-supercat

graceguo-supercat · 2020-01-10T21:12:03Z

airbnb is using Presto version 0.188.
this is the query that generates above error (not sure how other ppl can reproduce):

SELECT
    dms.ds_week_ending
  , dms.dim_affiliate_channel
  , dms.campaign_strategy
  , CONCAT(dms.dim_affiliate_channel, '_', dms.campaign_strategy) AS dim_group_id
    , (SUM(dms.bookings) - SUM(ba.predicted_bookings)) / SUM(dms.bookings) AS bias_m_bookings
    , (SUM(dms.nights) - SUM(ba.predicted_nights)) / SUM(dms.nights) AS bias_m_nights
    , (SUM(dms.incremental_cm) - SUM(ba.predicted_incremental_cm)) / SUM(dms.incremental_cm) AS bias_m_incremental_cm
    , SUM(dms.incremental_cm) / SUM(dms.cost) - SUM(ba.predicted_incremental_cm) / SUM(dms.cost) AS bias_efficiency
--   , CASE
--       WHEN dms.account_name in ('DEMAND:FIN','DEMAND:RUS-ROW','DEMAND:SPA-COL','DEMAND:SWE','DEMAND:POL','DEMAND:NOR','DEMAND:GER-AUT','DEMAND:HUN','DEMAND:HRV','DEMAND:CZE','DEMAND:SPA-URY','DEMAND:SPA-PER','DEMAND:THA','DEMAND:HEB','DEMAND:IDN','DEMAND:ARA','DEMAND:MAY','DEMAND:GRE','DEMAND:SRB','DEMAND:EST','DEMAND:UKR','DEMAND:CAT','DEMAND:SPA-MEX','DEMAND:SVN','DEMAND:BGR','DEMAND:ROU','DEMAND:SVK','DEMAND:LVA','DEMAND:VNM','DEMAND:LTU') THEN 'Other'
--       ELSE dms.account_name END AS account_name
--   , 'All Devices' as device
--   , dms.experiment_label
--   , dms.bid_system
--   , dms.campaign_strategy
--   , dms.match_type
--   , dms.domestic_intl
FROM
(
  SELECT
    ft.ds_week_ending
  , dms.calendar_date
  , dms.dim_affiliate_channel
  , dms.platform_id
  , dms.account_name as account_name
  , dms.campaign_name
  , dms.dim_external_campaign_id
--   , CASE
--         WHEN dms.account_name in ('DEMAND:ENG-ROW:TEST','DEMAND:SPA-ARG:TEST') THEN 'NewEngen Bidding'
--         WHEN c.bidding_strategy_type = 'Target ROAS' THEN 'TROAS'
--         WHEN c.bidding_strategy_type in ('cpc', 'Enhanced CPC') AND c.labels LIKE '%airbidder_eligible%' AND c.labels NOT LIKE '%exclude-auto-bidding%' THEN 'airbidder_core'
--         WHEN c.bidding_strategy_type in ('cpc', 'Enhanced CPC') AND c.labels LIKE '%exclude-auto-bidding%' THEN 'exclude-auto-bidding'
--         WHEN c.labels LIKE '%TEST_Q32018_BMM_AIRBIDDER_CONTROL%' THEN 'BMM_AIRBIDDER_CONTROL'
--         WHEN c.labels LIKE '%TEST_Q32018_BMM_AIRBIDDER_TREATMENT%' THEN 'BMM_AIRBIDDER_TREATMENT'
--         WHEN c.labels LIKE '%TEST_Q32018_ONEMODEL_CONTROL%' THEN 'ONEMODEL_CONTROL'
--         WHEN c.labels LIKE '%TEST_Q32018_ONEMODEL_TREATMENT%' THEN 'ONEMODEL_TREATMENT'
--         WHEN c.labels LIKE '%TEST_Q32018_C2B_V3_CONTROL%' THEN 'C2BV3_CONTROL'
--         WHEN c.labels LIKE '%TEST_Q32018_C2B_V3_TREATMENT%' THEN 'C2BV3_TREATMENT'
--         WHEN c.bidding_strategy_type in ('cpc', 'Enhanced CPC') AND dim_classifications['channel'] != 'SEM Non-brand' THEN 'Unlabeled Manual CPC'
--         ELSE 'Unknown' END AS bid_system
--   , CASE
--         WHEN dms.account_name in ('DEMAND:ENG-ROW:TEST','DEMAND:SPA-ARG:TEST') THEN 'NewEngen'
--         WHEN dms.campaign_name LIKE '%TEST_CM-User-August-2018' THEN 'USER_AUDIENCE'
--         WHEN c.labels LIKE '%TEST_Q32018_BMM_AIRBIDDER_CONTROL%' THEN 'BMM_AIRBIDDER_CONTROL'
--         WHEN c.labels LIKE '%TEST_Q32018_BMM_AIRBIDDER_TREATMENT%' THEN 'BMM_AIRBIDDER_TREATMENT'
--         WHEN c.labels LIKE '%TEST_Q32018_ONEMODEL_CONTROL%' THEN 'ONEMODEL_CONTROL'
--         WHEN c.labels LIKE '%TEST_Q32018_ONEMODEL_TREATMENT%' THEN 'ONEMODEL_TREATMENT'
--         WHEN c.labels LIKE '%TEST_Q32018_C2B_V3_CONTROL%' THEN 'C2BV3_CONTROL'
--         WHEN c.labels LIKE '%TEST_Q32018_C2B_V3_TREATMENT%' THEN 'C2BV3_TREATMENT'
--         ELSE 'not_in_experiments' END AS experiment_label
  , dim_classifications['campaign_strategy'] as campaign_strategy
--   , CASE
--       WHEN dms.campaign_name LIKE '%[EXACT]%' THEN 'EXACT'
--       WHEN dms.campaign_name LIKE '%[BMM]%' THEN 'BMM'
--       WHEN dms.campaign_name LIKE '%[PHRASE]%' THEN 'PHRASE'
--       WHEN dms.campaign_name LIKE '%:DSA%' THEN 'DSA'
--       ELSE 'Mixed' END AS match_type
--   , CASE
--     WHEN upper(dms.campaign_name) LIKE '%:COMP%' THEN 'N/A'
--     WHEN upper(dms.campaign_name) LIKE '%:DSA%' THEN 'N/A'
--     WHEN upper(dms.campaign_name) LIKE '%GENERIC%' THEN 'N/A'
--     WHEN upper(dms.campaign_name) LIKE '%KW:%' AND SPLIT(upper(dms.campaign_name),':')[1] = SUBSTR(SPLIT(upper(dms.campaign_name),'KW:')[2],1,3) THEN 'domestic'
--     WHEN upper(dms.campaign_name) LIKE '%DEST:%' AND SPLIT(upper(dms.campaign_name),':')[1] =  SUBSTR(SPLIT(upper(dms.campaign_name),'DEST:')[2],1,3) THEN 'domestic'
--     WHEN upper(dms.campaign_name) LIKE '%POI:%' AND SPLIT(upper(dms.campaign_name),':')[1] =  SUBSTR(SPLIT(upper(dms.campaign_name),'POI:')[2],1,3) THEN 'domestic'
--     WHEN dms.campaign_name LIKE '%SRC:%' AND SPLIT(dms.campaign_name,':')[1] =  SUBSTR(SPLIT(dms.campaign_name,'SRC:')[2],1,3) THEN 'domestic'
--     ELSE 'international' END AS domestic_intl
  , SUM(dms.m_cost) AS cost
  , SUM(m_bookings_by_guest_stages_cohorted['visitor'] + m_bookings_by_guest_stages_cohorted['user'] + m_bookings_by_guest_stages_cohorted['booker']) AS bookings
  , SUM(m_nights_by_guest_stages_cohorted['visitor'] + m_nights_by_guest_stages_cohorted['user'] + m_nights_by_guest_stages_cohorted['booker']) AS nights
  , SUM(m_incremental_cm_by_guest_stages_cohorted['visitor'] + m_incremental_cm_by_guest_stages_cohorted['user'] + m_incremental_cm_by_guest_stages_cohorted['booker']) AS incremental_cm
  FROM omg.dim_marketing_summary_v2_campaign_cohorted dms
  JOIN core_data.dim_time ft
    ON dms.calendar_date = ft.ds
  JOIN omg.campaign_performance_report_all_today c
    ON dms.dim_external_campaign_id = c.campaign_id
    AND dms.ds = c.ds
  WHERE
    -- returns calendar_date for Sunday week_start of 4 complete weeks ago.
    dms.calendar_date < CAST(date_add('day', -1,date_trunc('week', date_add('day', -21, CAST(dms.ds AS DATE)))) AS VARCHAR)
    AND dms.ds = '{{ presto.latest_partition("omg.dim_marketing_summary_v2_campaign_cohorted") }}'
    AND dms.platform_id = 0
    AND c.ad_network_type2 = 'Google search'
    AND dms.account_name LIKE '%DEMAND:%'
    -- AND dms.account_name in ('DEMAND:ENG-ROW:TEST','DEMAND:SPA-ARG:TEST')
    AND dms.account_name NOT LIKE '%REMARKETING%'
    AND dms.account_name NOT LIKE '%PROSPECTING%'
    AND dms.account_name NOT LIKE '%EXPERIENCES%'
    AND dms.account_name NOT LIKE '%ALL:XXM:SRC%'
    AND dms.dim_classifications['channel'] in ('SEM Non-brand','SEM Brand')
    AND dms.dim_classifications['campaign_strategy'] NOT IN ('SEM Brand') -- filter out duplicate dimensions which was causing double-counting of SEM Brand
    -- filters for QA
    -- AND dms.account_name = 'DEMAND:ENG-ROW:TEST'
    -- AND dms.account_name = 'DEMAND:FRE'
    -- AND dms.campaign_name = 'USA:DTM:SRC:KW:FRA+Paris[BMM]'
  GROUP BY 1,2,3,4,5,6,7,8
  ) as dms
JOIN (
  SELECT
    calendar_date
  , dim_affiliate_channel
  , platform_id
  , account_name
  , campaign_name
  , dim_external_campaign_id
  , SUM(m_cost) AS cost
  , SUM(predict_m_bookings) AS predicted_bookings
  , SUM(predict_m_nights) AS predicted_nights
  , SUM(predict_m_incremental_cm) AS predicted_incremental_cm
  FROM omg.dim_marketing_summary_baby_ace v2
  JOIN core_data.dim_time ft
    ON v2.calendar_date = ft.ds
  WHERE
    v2.ds >='2018-07-01'
    -- returns calendar_date for Sunday week_start of 4 complete weeks ago.
    AND day_of_week(CAST(v2.ds AS DATE)) = 6
    AND calendar_date >='2018-06-25'
    AND date_diff('day',CAST(calendar_date AS DATE), CAST(v2.ds AS DATE)) >= 0
    AND date_diff('day',CAST(calendar_date AS DATE), CAST(v2.ds AS DATE)) <= 6
    AND dim_affiliate_channel in ('sem-non-brand','sem-brand')
    AND platform_id = 0
    AND dim_group_name = 'channel_platform_account_campaign'
    AND dim_group_id LIKE '%0_DEMAND%'
    AND dim_group_id NOT LIKE '%:REMARKETING:%'
    AND dim_group_id NOT LIKE '%:PROSPECTING:%'
    AND dim_group_id NOT LIKE '%EXPERIENCES%'
    AND dim_group_id NOT LIKE '%ALL:XXM:SRC%'
    GROUP BY 1,2,3,4,5,6
) as ba
  ON dms.calendar_date = ba.calendar_date
  AND dms.dim_affiliate_channel = ba.dim_affiliate_channel
  AND dms.platform_id = ba.platform_id
  AND dms.account_name =ba.account_name
  AND dms.dim_external_campaign_id = ba.dim_external_campaign_id
--WHERE dms.bid_system not in ('exclude-auto-bidding','Unknown')
GROUP BY 1,2,3

Revert "Make select_star work with SQL Lab views (apache#8598)"

0837d74

This reverts commit 964e6db

pull-request-size bot added the size/M label Jan 6, 2020

graceguo-supercat requested review from betodealmeida and etr2460 and removed request for betodealmeida January 6, 2020 19:54

etr2460 approved these changes Jan 6, 2020

View reviewed changes

graceguo-supercat merged commit 93bcace into apache:master Jan 6, 2020

mistercrunch added the .revert Requires being reverted label Jan 6, 2020

graceguo-supercat deleted the gg-Revert8598 branch February 10, 2020 04:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "Make select_star work with SQL Lab views (#8598)" #8930

Revert "Make select_star work with SQL Lab views (#8598)" #8930

graceguo-supercat commented Jan 6, 2020 •

edited

etr2460 left a comment

codecov-io commented Jan 6, 2020

mistercrunch commented Jan 6, 2020

betodealmeida commented Jan 7, 2020

DiggidyDave commented Jan 10, 2020 •

edited

graceguo-supercat commented Jan 10, 2020 •

edited

Revert "Make select_star work with SQL Lab views (#8598)" #8930

Revert "Make select_star work with SQL Lab views (#8598)" #8930

Conversation

graceguo-supercat commented Jan 6, 2020 • edited

CATEGORY

SUMMARY

TEST PLAN

ADDITIONAL INFORMATION

REVIEWERS

etr2460 left a comment

Choose a reason for hiding this comment

codecov-io commented Jan 6, 2020

Codecov Report

mistercrunch commented Jan 6, 2020

betodealmeida commented Jan 7, 2020

DiggidyDave commented Jan 10, 2020 • edited

graceguo-supercat commented Jan 10, 2020 • edited

graceguo-supercat commented Jan 6, 2020 •

edited

DiggidyDave commented Jan 10, 2020 •

edited

graceguo-supercat commented Jan 10, 2020 •

edited