fix(ingestion/lookml): liquid template resolution and use datahub sqlparser for column level lineage #10542

sid-acryl · 2024-05-20T07:02:32Z

Update code to use DataHub SqlParser for SQL parsing
And fixes issues in CLL generation when view definition language is SQL
Added support for liquid template resolution for lookml views

hsheth2 · 2024-05-24T19:04:38Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_sql_parser.py

+        syntax_func_map: Dict[str, Callable] = {
+            "syntax1": partial(_update_fields, self.fields, spr),
+            "syntax2": partial(_create_fields, spr),
+        }


this looks pretty weird - can we just use an if statement instead of this whole "syntax1/2" thing

Updated this section

hsheth2 · 2024-05-24T19:10:04Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_source.py

@@ -1295,64 +1299,28 @@ def _extract_metadata_from_derived_table_sql(
        # Skip queries that contain liquid variables. We currently don't parse them correctly.
        # Docs: https://cloud.google.com/looker/docs/liquid-variable-reference.
        # TODO: also support ${EXTENDS} and ${TABLE}
-        if "{%" in sql_query:


where does all the jinja-related logic go now?

In this commit I updated the old code to as per syntax, will add complete code in next commit.

…hub-fork into master+ing-510-lookml-cll

hsheth2 · 2024-05-28T18:47:28Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_source.py

@@ -1673,7 +1595,30 @@ def _get_upstream_lineage(
                                make_schema_field_urn(
                                    upstream_dataset_urn, upstream_field
                                )
-                                for upstream_field in field.upstream_fields
+                                for upstream_field in cast(
+                                    List[str], field.upstream_fields


why is this cast required?

I updated the ViewField.upstream_fields from List[str] to upstream_fields: Union[List[str], List[ColumnRef]]. Because it is either a column name in-case of non-sql lineage or ColumnRef for sql query parsed result

why can't it always be List[ColumnRef]? we can convert the strings to ColumnRef so that it's all uniform

hsheth2 · 2024-05-28T19:10:14Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_sql_parser.py


    return fields


+def get_qt_name(urn: str) -> str:


It is get_qualified_table_name.

hsheth2 · 2024-05-28T19:11:25Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_sql_parser.py

+        Undefined.__str__ = lambda instance: "NULL"  # type: ignore
+
+        # Resolve liquid template
+        return Template(sql_query).render(self.liquid_context)


the user should also be able to pass in a variable list into the ingestion config

That's the point I asked you on slack , do we really need variable list if NULL can work? , let me check other scenario as discussed over slack.

Updated the code to pass liquid_variable as config

hsheth2 · 2024-05-28T19:13:02Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_source.py

+                            upstreams=[
+                                make_schema_field_urn(cll_ref.table, cll_ref.column)
+                                for cll_ref in cast(
+                                    List[ColumnRef], field.upstream_fields


why is this cast required?

in general, anytime you're using cast, there's a decent chance you're doing something wrong or need an if thing is None check

Please check ViewField.upstream_fields type

…hub-fork into master+ing-510-lookml-cll

hsheth2 · 2024-06-11T22:45:36Z

metadata-ingestion/tests/integration/lookml/lkml_samples_hive/liquid.view.lkml

@@ -7,7 +7,11 @@ sql:
      FROM
        order
      WHERE
-        {% condition order_region %} order.region {% endcondition %}


why was this changed?

python-liquid template was raising error for condition as it is not liquid tag, it is specific to looker and later as it was present in sql, the parser was raising parsing error for this.

I updated this section to use liquid conditional logic, see the updated file lkml_samples_hive/liquid.view.lkml

hsheth2 · 2024-06-11T22:48:39Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_sql_parser.py


        spr: SqlParsingResult = create_lineage_sql_parsed_result(
            query=query,
            default_schema=connection.default_schema,
            default_db=connection.default_db,
            platform=connection.platform,
            platform_instance=connection.platform_instance,
-            env=env,
-            graph=ctx.graph,
+            env=cast(str, connection.platform_env),  # It's never going to be None


if this is true, let's just change the type of connection.platform_env to not be optional

It will break existing recipe, please check this L108 and as per below comments it is never going to be None as we are setting it to env of looker

hsheth2 · 2024-06-11T22:54:17Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_source.py

+        # LookerConnectionDefinition.platform_env
+        if connection_def:
+            if connection_def.platform_env is None:
+                connection_def.platform_env = self.source_config.env


this doesn't seem right

Pleaser read this field description #L178 and no where we were making sure that it should happened. It was crashing the sql parser, not able to recall exact issue, but it setting env to None

hsheth2 · 2024-06-11T22:54:58Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_source.py

@@ -1673,7 +1595,30 @@ def _get_upstream_lineage(
                                make_schema_field_urn(
                                    upstream_dataset_urn, upstream_field
                                )
-                                for upstream_field in field.upstream_fields
+                                for upstream_field in cast(
+                                    List[str], field.upstream_fields


why can't it always be List[ColumnRef]? we can convert the strings to ColumnRef so that it's all uniform

hsheth2 · 2024-06-11T22:58:27Z

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_source.py

@@ -2120,6 +1271,14 @@ def get_internal_workunits(self) -> Iterable[MetadataWorkUnit]:  # noqa: C901

            project_name = self.get_project_name(model_name)

+            looker_view_id_cache: LookerViewIdCache = LookerViewIdCache(


what is this used for?

It is for the case where view is referred in SQL statement as ${view-name.SQL_TABLE_NAME}.
Check this view top_10_employee_income_source.view.lkml.

In this derived sql case at L401 we are getting urn with .sql_table_name suffix and platform, database and schema as per setting available in connection, however these things are not true for derived view

At this line L417, we are overwriting such urn by actual view-urn.

This cache is helping to find out LookerViewId so that we can generate the view urn and also keeping the LookerViewId in cache

…hub-fork into master+ing-510-lookml-cll

sid-acryl added 2 commits May 17, 2024 20:30

wip

32a6ab0

introduce datahub sqlparser for sql parsing

b308f91

github-actions bot added the ingestion PR or Issue related to the ingestion of metadata label May 20, 2024

remove import pdb

d09b296

vercel bot deployed to Preview May 20, 2024 07:31 View deployment

sid-acryl and others added 4 commits May 24, 2024 15:27

Merge branch 'master' into master+ing-510-lookml-cll

3010df0

syntax1 and syntax2 support of lookml sql view

4dedd87

update message

7922cc7

Merge branch 'master' into master+ing-510-lookml-cll

86026c6

vercel bot had a problem deploying to Preview May 24, 2024 17:37 Failure

hsheth2 reviewed May 24, 2024

View reviewed changes

sid-acryl added 2 commits May 27, 2024 22:57

add if else for syntax

9f9d510

Merge branch 'master+ing-510-lookml-cll' of github.com:sid-acryl/data…

93add15

…hub-fork into master+ing-510-lookml-cll

vercel bot had a problem deploying to Preview May 27, 2024 17:36 Failure

liquid variable resolution

9dbd32b

vercel bot deployed to Preview May 28, 2024 15:09 View deployment

hsheth2 reviewed May 28, 2024

View reviewed changes

sid-acryl and others added 2 commits May 29, 2024 21:49

liquid variable in config

4f44fd9

Merge branch 'master' into master+ing-510-lookml-cll

fe96655

vercel bot deployed to Preview May 29, 2024 16:50 View deployment

Merge branch 'master' into master+ing-510-lookml-cll

522d4a5

vercel bot deployed to Preview May 30, 2024 06:22 View deployment

sid-acryl added 2 commits May 30, 2024 14:05

sqlglot_lib

fcd9957

Merge branch 'master+ing-510-lookml-cll' of github.com:sid-acryl/data…

808ded1

…hub-fork into master+ing-510-lookml-cll

vercel bot deployed to Preview May 30, 2024 08:49 View deployment

test fixes

542de95

vercel bot deployed to Preview June 1, 2024 03:56 View deployment

view to view lineage

ab61277

vercel bot deployed to Preview June 5, 2024 08:29 View deployment

Merge branch 'master' into master+ing-510-lookml-cll

734fbbe

vercel bot deployed to Preview June 5, 2024 12:10 View deployment

sid-acryl added 2 commits June 6, 2024 17:43

drop hive. from urn id

7e75198

Merge branch 'master+ing-510-lookml-cll' of github.com:sid-acryl/data…

3d2b13b

…hub-fork into master+ing-510-lookml-cll

vercel bot deployed to Preview June 6, 2024 12:28 View deployment

name.SQL_TABLE_NAME handling

2e6bffd

vercel bot deployed to Preview June 7, 2024 03:33 View deployment

sid-acryl added 2 commits June 7, 2024 18:55

added debug log

5f9a582

add stack trace

dc231a6

vercel bot deployed to Preview June 7, 2024 13:54 View deployment

sid-acryl marked this pull request as ready for review June 7, 2024 18:33

fix for select * from something.SQL_TABLE_NAME

4cc0048

vercel bot deployed to Preview June 11, 2024 05:40 View deployment

fix test cases

54aff14

vercel bot deployed to Preview June 11, 2024 13:32 View deployment

sid-acryl changed the title ~~fix(ingestion/looker): use datahub sqlparser for column level lineage~~ fix(ingestion/looker): use datahub sqlparser for column level lineage and liquid template resolution Jun 11, 2024

sid-acryl changed the title ~~fix(ingestion/looker): use datahub sqlparser for column level lineage and liquid template resolution~~ fix(ingestion/looker): liquid template resolution and use datahub sqlparser for column level lineage Jun 11, 2024

sid-acryl changed the title ~~fix(ingestion/looker): liquid template resolution and use datahub sqlparser for column level lineage~~ fix(ingestion/lookml): liquid template resolution and use datahub sqlparser for column level lineage Jun 11, 2024

hsheth2 reviewed Jun 11, 2024

View reviewed changes

sid-acryl and others added 2 commits June 12, 2024 10:19

test case

6c79b37

Merge branch 'master' into master+ing-510-lookml-cll

b77eb3f

vercel bot deployed to Preview June 12, 2024 05:19 View deployment

sid-acryl added 2 commits June 12, 2024 13:36

rephrased description

d58600f

Merge branch 'master+ing-510-lookml-cll' of github.com:sid-acryl/data…

1415b10

…hub-fork into master+ing-510-lookml-cll

vercel bot deployed to Preview June 12, 2024 08:24 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ingestion/lookml): liquid template resolution and use datahub sqlparser for column level lineage #10542

fix(ingestion/lookml): liquid template resolution and use datahub sqlparser for column level lineage #10542

sid-acryl commented May 20, 2024 •

edited

hsheth2 May 24, 2024

sid-acryl May 27, 2024

hsheth2 May 24, 2024

sid-acryl May 27, 2024

hsheth2 May 28, 2024

sid-acryl May 29, 2024

hsheth2 Jun 11, 2024

hsheth2 May 28, 2024

sid-acryl May 29, 2024 •

edited

hsheth2 May 28, 2024

sid-acryl May 29, 2024

sid-acryl May 29, 2024

hsheth2 May 28, 2024

sid-acryl May 29, 2024

hsheth2 Jun 11, 2024

sid-acryl Jun 12, 2024

hsheth2 Jun 11, 2024

sid-acryl Jun 12, 2024 •

edited

hsheth2 Jun 11, 2024

sid-acryl Jun 12, 2024 •

edited

hsheth2 Jun 11, 2024

hsheth2 Jun 11, 2024

sid-acryl Jun 12, 2024

		@@ -2120,6 +1271,14 @@ def get_internal_workunits(self) -> Iterable[MetadataWorkUnit]: # noqa: C901

		project_name = self.get_project_name(model_name)

		looker_view_id_cache: LookerViewIdCache = LookerViewIdCache(

fix(ingestion/lookml): liquid template resolution and use datahub sqlparser for column level lineage #10542

Are you sure you want to change the base?

fix(ingestion/lookml): liquid template resolution and use datahub sqlparser for column level lineage #10542

Conversation

sid-acryl commented May 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sid-acryl May 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sid-acryl Jun 12, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sid-acryl Jun 12, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sid-acryl commented May 20, 2024 •

edited

sid-acryl May 29, 2024 •

edited

sid-acryl Jun 12, 2024 •

edited

sid-acryl Jun 12, 2024 •

edited