fix(ingest/powerbi): support each expression in m-query function invocation #7541
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
ingestion
hsheth2
reviewed
Mar 10, 2023
| @@ -100,6 +100,8 @@ combine_result | |||
|
|
|||
| `Pattern-1` is supported as it first assign the table from schema to variable and then variable is used in M-Query Table function i.e. Table.Combine | |||
|
|
|||
| Note: If you are using (PowerBI Parameters)[https://learn.microsoft.com/en-us/power-query/power-query-query-parameters]. then avoid using same name for variable in M-Query | |||
There was a problem hiding this comment.
we should also add the caveat that we don't support parameters with the admin API
There was a problem hiding this comment.
Added it in Admin Ingestion vs. Basic Ingestion
metadata-ingestion/src/datahub/ingestion/source/powerbi/m_query/tree_function.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi-lexical-grammar.rule
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi-lexical-grammar.rule
Outdated
Show resolved
Hide resolved
hsheth2
reviewed
Mar 10, 2023
…dsiddique/datahub into master+fix-powerbi-bigquery-datasets
hsheth2
requested changes
Mar 13, 2023
metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi-lexical-grammar.rule
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi-lexical-grammar.rule
Outdated
Show resolved
Hide resolved
jjoyce0510
approved these changes
Mar 15, 2023
hsheth2
approved these changes
Mar 15, 2023
jjoyce0510
reviewed
Mar 15, 2023
| @@ -304,6 +305,7 @@ unary_expression: type_expression | |||
| | "+" unary_expression | |||
| | "_" unary_expression | |||
| | "not" unary_expression | |||
| | expression | |||
There was a problem hiding this comment.
nice - so this broadens the definition so that unary expression can be any type of expression?
There was a problem hiding this comment.
yep - this isn't strictly correct because it can cause incorrect order of operations in the AST (e.g. addition before multiplication or something), but we don't really care about that here
jjoyce0510
reviewed
Mar 15, 2023
| @@ -432,17 +434,7 @@ optional_parameter: "optional" WS_INLINE parameter | |||
|
|
|||
| each_expression: "each" WS_INLINE each_expression_body | |||
|
|
|||
| // relational_expression rule is different then relational expression used in below grammar to resolve "each" construct | |||
| each_expression_body: function_body | |||
There was a problem hiding this comment.
in simple terms - why are we able to remove this?
There was a problem hiding this comment.
the addition to unary operator makes this one work correctly
jjoyce0510
reviewed
Mar 15, 2023
| # This is the case where they reference a variable using | ||
| # the `#"Name of variable"` syntax. | ||
| # the `#"Name of variable"` or `Variable` syntax. It can be |
There was a problem hiding this comment.
What was the previous assert doing here? No purpose?
There was a problem hiding this comment.
this broadens it from quoted_identifier -> identifier (which includes regular_identifier)
hsheth2
changed the title
shirshanka
pushed a commit
to shirshanka/datahub
that referenced
this pull request
Mar 22, 2023
…cation (datahub-project#7541) Co-authored-by: MohdSiddiqueBagwan <mohdsiddique.bagwan@gslab.com> Co-authored-by: Harshal Sheth <hsheth2@gmail.com>
shirshanka
pushed a commit
to shirshanka/datahub
that referenced
this pull request
Mar 22, 2023
…cation (datahub-project#7541) Co-authored-by: MohdSiddiqueBagwan <mohdsiddique.bagwan@gslab.com> Co-authored-by: Harshal Sheth <hsheth2@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Quick fixes after PR: #7502