Correctly escape query used with Redshift UNLOAD

[rizzatti]

Description

SQL queries used in the context of the UNLOAD command in Redshift need to have any single quotes escaped.

This change fixes a little bug which didn't correctly add the backslashes to the query string.

Have you tested this? If so, how?

While creating some jobs that use RedshiftUnloadTask earlier today, I noticed the issue. This PR fixes it.

[mention-bot]

@rizzatti, thanks for your PR! By analyzing the history of the files in this pull request, we identified @chenzhan, @ddaniels888 and @steenzout to be potential reviewers.

[Tarrasch]

Can you add a testcase?

[rizzatti]

@Tarrasch I fixed the current testcase.
It's easy to notice the problem by looking at the test errors from Travis' last run. The expected command:

AssertionError: Expected call: execute("UNLOAD ( 'SELECT 'a' as col_a, current_date as col_b' ) TO 's3://bucket/key' credentials 'aws_access_key_id=AWS_ACCESS_KEY;aws_secret_access_key=AWS_SECRET_KEY' DELIMITER ',' ADDQUOTES GZIP ALLOWOVERWRITE PARALLEL OFF;")

The quoted query 'SELECT 'a' as col_a, current_date as col_b' would be misinterpreted due to the quotes around the 'a' not being properly escaped.

[Tarrasch]

Can you get a redshift person to review this? :)

[rizzatti]

@Tarrasch, would this help?

[rizzatti]

These are the contents of example.py in the screenshots above.

# -*- coding: utf-8 -*-

import os
import luigi
import luigi.contrib.redshift


class RedshiftUnloadExample(luigi.contrib.redshift.RedshiftUnloadTask):

    host = os.environ.get('RS_HOST')
    user = os.environ.get('RS_USER')
    password = os.environ.get('RS_PASS')
    database = os.environ.get('RS_DATABASE')
    table = os.environ.get('RS_TABLE')
    aws_access_key_id = os.environ.get('AWS_ACCESS_KEY_ID')
    aws_secret_access_key = os.environ.get('AWS_SECRET_ACCESS_KEY')
    s3_unload_path = os.environ.get('RS_UNLOAD_PATH')

    # This comes straight from test/contrib/redshift_test.py
    def query(self):
        return "SELECT 'a' as col_a, current_date as col_b"


if __name__ == '__main__':
    luigi.run()

[Tarrasch]

@rizzatti, I (or any other single volunteering maintainer) cannot be expected to understand details of every system luigi interoperates. Please find another reviewer.

[dlstadther]

I don't systematically use the UNLOAD function in my ETL so I haven't tested this myself. But assuming it worked previously and the only case it failed was when ' was used within the unload query, then I don't see anything wrong with this update to escaping '.

[Tarrasch]

Thanks for review! :)

[rizzatti]

Thanks guys.