Skip to content

Conversation

@wendigo
Copy link
Contributor

@wendigo wendigo commented Feb 4, 2025

Description

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@wendigo wendigo requested a review from martint February 4, 2025 21:36
@cla-bot cla-bot bot added the cla-signed label Feb 4, 2025
@mosabua
Copy link
Member

mosabua commented Feb 4, 2025

Can you also expand the docs a bit .. link to it from session properties doc and add an example somewhere in the selecd.md

@wendigo
Copy link
Contributor Author

wendigo commented Feb 4, 2025

Alternatively:

queryScoped
    :   (WITH SESSION sessionProperty (',' sessionProperty)*)
      | (WITH functionSpecification (',' functionSpecification)*)
      | (WITH SESSION sessionProperty (',' sessionProperty)* functionSpecification (',' functionSpecification)*)
    ;

as we have three distinct cases:

WITH session functions
WITH session
WITH functions

@wendigo
Copy link
Contributor Author

wendigo commented Feb 4, 2025

let me know @martint which option do you prefer

@wendigo wendigo force-pushed the serafin/grammar-unambigous branch from d98df14 to 22c638e Compare February 4, 2025 22:04
@wendigo wendigo force-pushed the serafin/grammar-unambigous branch from 2e9eea5 to dafc6f9 Compare February 5, 2025 18:45
Copy link
Member

@martint martint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're going to have to adjust the test in TestSqlParser.testWithSessionAndFunction() accordingly

@wendigo wendigo force-pushed the serafin/grammar-unambigous branch 2 times, most recently from b389ba3 to 6c99002 Compare February 5, 2025 18:59
@github-actions github-actions bot added the docs label Feb 5, 2025
@wendigo wendigo force-pushed the serafin/grammar-unambigous branch from 6c99002 to 2941983 Compare February 5, 2025 19:10
@wendigo wendigo requested a review from martint February 5, 2025 19:10
@wendigo wendigo force-pushed the serafin/grammar-unambigous branch 2 times, most recently from 0719941 to 452bbe7 Compare February 5, 2025 19:24
This is unambigous if the WITH SESSION always
appears before WITH FUNCTION as the specified
session properties are always visible to the
function definition and during function execution
in the query body.
@wendigo wendigo force-pushed the serafin/grammar-unambigous branch from 452bbe7 to c4325f7 Compare February 5, 2025 20:08
@wendigo wendigo merged commit 88fca50 into trinodb:master Feb 5, 2025
87 of 97 checks passed
@wendigo wendigo deleted the serafin/grammar-unambigous branch February 5, 2025 20:41
@github-actions github-actions bot added this to the 470 milestone Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants