Expand SQL formatter to LakeFormation

[LeonLuttenberger]

Feature or Bugfix

• Feature

Detail

• Expand SQL formatter to LakeFormation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
• Commit ID: 23cdef4
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
• Commit ID: 23cdef4
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
• Commit ID: 23cdef4
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[jaidisido]

I believe LakeFormation uses Redshift Spectrum under the hood so I don't believe the Presto Engine definition we have would work for conditions like IN. Could we please just check that we don't need to create a new Engine definition for it? Looks good otherwise, thanks!

[LeonLuttenberger]

I believe LakeFormation uses Redshift Spectrum under the hood so I don't believe the Presto Engine definition we have would work for conditions like IN. Could we please just check that we don't need to create a new Engine definition for it? Looks good otherwise, thanks!

@jaidisido The only difference I found between how the engines handle literals was that in how Presto and Hive escape string quotations. However, PartiQL does it the same way that Presto does (' within a string is escaped as '').

I'm not sure what you mean by IN though? Presto and PartiQL seem to have the same syntax for IN (values separated by commas enclosed by parenthesis). There is an issue here thought that customers have no way of passing us a list of parameters that go into IN, and have us parse it as such.

For example, if they give us

sql="SELECT * FROM table WHERE id IN :ids",
params={"ids": [1, 2, 3]}

we will parse this as SELECT * FROM table WHERE id IN ARRAY[1, 2, 3]. This query would therefore be invalid for both Presto and PartiQL. I'm not sure how to solve this gap though, as we need to parse lists as arrays.

[malachi-constant]

LGTM, holding off on approval until discussion started by @jaidisido is resovled.

[jaidisido]

I believe LakeFormation uses Redshift Spectrum under the hood so I don't believe the Presto Engine definition we have would work for conditions like IN. Could we please just check that we don't need to create a new Engine definition for it? Looks good otherwise, thanks!

@jaidisido The only difference I found between how the engines handle literals was that in how Presto and Hive escape string quotations. However, PartiQL does it the same way that Presto does (' within a string is escaped as '').

I'm not sure what you mean by IN though? Presto and PartiQL seem to have the same syntax for IN (values separated by commas enclosed by parenthesis). There is an issue here thought that customers have no way of passing us a list of parameters that go into IN, and have us parse it as such.

For example, if they give us

sql="SELECT * FROM table WHERE id IN :ids",
params={"ids": [1, 2, 3]}

we will parse this as SELECT * FROM table WHERE id IN ARRAY[1, 2, 3]. This query would therefore be invalid for both Presto and PartiQL. I'm not sure how to solve this gap though, as we need to parse lists as arrays.

The Array one is tricky indeed, but I meant for even simpler examples like dt.datetime, this would parse to TIMESTAMP '...' which I don't believe would be accepted in a partiql LakeFormation/Redshift query. I think we need to test this formatter against some actual queries submitted to the engine to determine our gaps

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
• Commit ID: 3e0d7ec
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
• Commit ID: 3e0d7ec
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
• Commit ID: 3e0d7ec
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
• Commit ID: d0f375a
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
• Commit ID: d0f375a
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[malachi-constant]

AWS CodeBuild CI Report

• CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
• Commit ID: d0f375a
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[jaidisido]

It looks great, thank you for taking the time to deep dive partiql!