Skip to content

Commit db63921

Browse files
authored
Adding patch to avoid alias conflict issues in join conditions (#425)
Adds a patch to the SQLGlot generation of join conditions to ensure aliases are _always_ maintained, especially in ambiguous cases. Also adjusts several facets of the synthea/wdi testing infrastructure to ensure the tests run as expected.
1 parent cc37e63 commit db63921

21 files changed

+24629
-258
lines changed

pydough/sqlglot/sqlglot_relational_visitor.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,9 +436,7 @@ def visit_join(self, join: Join) -> None:
436436
this=inputs[1],
437437
alias=TableAlias(this=alias_map.get(join.default_input_aliases[i], None)),
438438
)
439-
cond: RelationalExpression = join.condition.accept_shuttle(
440-
self._alias_remover
441-
).accept_shuttle(self._alias_modifier)
439+
cond: RelationalExpression = join.condition.accept_shuttle(self._alias_modifier)
442440
cond_expr: SQLGlotExpression = self._expr_visitor.relational_to_sqlglot(cond)
443441
join_type: str = join.join_type.value
444442
if join_type == "SEMI" and join.cardinality.singular:

tests/conftest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ def sqlite_custom_datasets_connection() -> DatabaseContext:
601601
"cd tests/gen_data",
602602
"rm -fv synthea.db",
603603
"rm -fv world_development_indicators.db",
604-
"sqlite3 synthea.db < init_synthea.sql",
604+
"sqlite3 synthea.db < init_synthea_sqlite.sql",
605605
"sqlite3 world_development_indicators.db < init_world_indicators_sqlite.sql",
606606
]
607607
# Get the shell commands required to re-create all the db files

tests/gen_data/init_world_indicators_sqlite.sql

Lines changed: 24451 additions & 121 deletions
Large diffs are not rendered by default.

tests/test_metadata/synthea_graph.json

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
{
77
"name": "all_prevalences",
88
"type": "simple table",
9-
"table path": "main.all_prevalences",
9+
"table path": "synthea.all_prevalences",
1010
"unique properties": [
1111
"ITEM"
1212
],
@@ -92,7 +92,7 @@
9292
{
9393
"name": "allergies",
9494
"type": "simple table",
95-
"table path": "main.allergies",
95+
"table path": "synthea.allergies",
9696
"unique properties": [
9797
"PATIENT",
9898
"ENCOUNTER",
@@ -182,7 +182,7 @@
182182
{
183183
"name": "careplans",
184184
"type": "simple table",
185-
"table path": "main.careplans",
185+
"table path": "synthea.careplans",
186186
"unique properties": [
187187
[
188188
"ID",
@@ -321,7 +321,7 @@
321321
{
322322
"name": "claims",
323323
"type": "simple table",
324-
"table path": "main.claims",
324+
"table path": "synthea.claims",
325325
"unique properties": [
326326
"ID"
327327
],
@@ -420,7 +420,7 @@
420420
{
421421
"name": "conditions",
422422
"type": "simple table",
423-
"table path": "main.conditions",
423+
"table path": "synthea.conditions",
424424
"unique properties": [
425425
[
426426
"START",
@@ -517,7 +517,7 @@
517517
{
518518
"name": "encounters",
519519
"type": "simple table",
520-
"table path": "main.encounters",
520+
"table path": "synthea.encounters",
521521
"unique properties": [
522522
"ID"
523523
],
@@ -620,7 +620,7 @@
620620
{
621621
"name": "immunizations",
622622
"type": "simple table",
623-
"table path": "main.immunizations",
623+
"table path": "synthea.immunizations",
624624
"unique properties": [
625625
"DATE",
626626
"PATIENT",
@@ -698,7 +698,7 @@
698698
{
699699
"name": "medications",
700700
"type": "simple table",
701-
"table path": "main.medications",
701+
"table path": "synthea.medications",
702702
"unique properties": [
703703
"START",
704704
"PATIENT",
@@ -817,7 +817,7 @@
817817
{
818818
"name": "observations",
819819
"type": "simple table",
820-
"table path": "main.observations",
820+
"table path": "synthea.observations",
821821
"unique properties": [
822822
[
823823
"DATE",
@@ -928,7 +928,7 @@
928928
{
929929
"name": "patients",
930930
"type": "simple table",
931-
"table path": "main.patients",
931+
"table path": "synthea.patients",
932932
"unique properties": [
933933
"patient"
934934
],
@@ -1155,7 +1155,7 @@
11551155
{
11561156
"name": "procedures",
11571157
"type": "simple table",
1158-
"table path": "main.procedures",
1158+
"table path": "synthea.procedures",
11591159
"unique properties": [
11601160
[
11611161
"DATE",

tests/test_metadata/world_development_indicators_graph.json

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
{
77
"name": "Country",
88
"type": "simple table",
9-
"table path": "main.Country",
9+
"table path": "wdi.Country",
1010
"unique properties": [
1111
"CountryCode"
1212
],
@@ -421,7 +421,7 @@
421421
{
422422
"name": "CountryNotes",
423423
"type": "simple table",
424-
"table path": "main.CountryNotes",
424+
"table path": "wdi.CountryNotes",
425425
"unique properties": [
426426
"Countrycode",
427427
"Seriescode"
@@ -473,7 +473,7 @@
473473
{
474474
"name": "Footnotes",
475475
"type": "simple table",
476-
"table path": "main.Footnotes",
476+
"table path": "wdi.Footnotes",
477477
"unique properties": [
478478
"Countrycode",
479479
"Seriescode",
@@ -539,7 +539,7 @@
539539
{
540540
"name": "Indicators",
541541
"type": "simple table",
542-
"table path": "main.Indicators",
542+
"table path": "wdi.Indicators",
543543
"unique properties": [
544544
"CountryCode",
545545
"IndicatorCode",
@@ -631,7 +631,7 @@
631631
{
632632
"name": "Series",
633633
"type": "simple table",
634-
"table path": "main.Series",
634+
"table path": "wdi.Series",
635635
"unique properties": [
636636
"SeriesCode"
637637
],
@@ -903,7 +903,7 @@
903903
{
904904
"name": "SeriesNotes",
905905
"type": "simple table",
906-
"table path": "main.SeriesNotes",
906+
"table path": "wdi.SeriesNotes",
907907
"unique properties": [
908908
"Seriescode",
909909
"Year"
@@ -966,7 +966,7 @@
966966
"CountryCode"
967967
]
968968
},
969-
"description": "Relationship between CountryNotes and Country",
969+
"description": "The country that the country note belongs to.",
970970
"synonyms": []
971971
},
972972
{
@@ -976,7 +976,7 @@
976976
"original property": "Country",
977977
"singular": false,
978978
"always matches": true,
979-
"description": "Relationship between and ",
979+
"description": "The notes for each country.",
980980
"synonyms": []
981981
},
982982
{
@@ -991,7 +991,7 @@
991991
"SeriesCode"
992992
]
993993
},
994-
"description": "Relationship between CountryNotes and Series",
994+
"description": "The series for each country note.",
995995
"synonyms": []
996996
},
997997
{
@@ -1001,7 +1001,7 @@
10011001
"original property": "Series",
10021002
"singular": false,
10031003
"always matches": true,
1004-
"description": "Relationship between and ",
1004+
"description": "The country notes for each series.",
10051005
"synonyms": []
10061006
},
10071007
{
@@ -1016,7 +1016,7 @@
10161016
"CountryCode"
10171017
]
10181018
},
1019-
"description": "Relationship between Footnotes and Country",
1019+
"description": "The country that the footnote belongs to.",
10201020
"synonyms": []
10211021
},
10221022
{
@@ -1026,7 +1026,7 @@
10261026
"original property": "Country",
10271027
"singular": false,
10281028
"always matches": true,
1029-
"description": "Relationship between and ",
1029+
"description": "The footnotes for the country.",
10301030
"synonyms": []
10311031
},
10321032
{
@@ -1041,7 +1041,7 @@
10411041
"SeriesCode"
10421042
]
10431043
},
1044-
"description": "Relationship between Footnotes and Series",
1044+
"description": "The Series that the footnote belongs to.",
10451045
"synonyms": []
10461046
},
10471047
{
@@ -1050,8 +1050,8 @@
10501050
"original parent": "Footnotes",
10511051
"original property": "Series",
10521052
"singular": false,
1053-
"always matches": true,
1054-
"description": "Relationship between and ",
1053+
"always matches": false,
1054+
"description": "The footnotes of the series.",
10551055
"synonyms": []
10561056
},
10571057
{
@@ -1066,7 +1066,7 @@
10661066
"CountryCode"
10671067
]
10681068
},
1069-
"description": "Relationship between Indicators and Country",
1069+
"description": "The country that the indicator belongs to.",
10701070
"synonyms": []
10711071
},
10721072
{
@@ -1076,7 +1076,7 @@
10761076
"original property": "Country",
10771077
"singular": false,
10781078
"always matches": true,
1079-
"description": "Relationship between and ",
1079+
"description": "The indicators for the country.",
10801080
"synonyms": []
10811081
},
10821082
{
@@ -1091,7 +1091,7 @@
10911091
"SeriesCode"
10921092
]
10931093
},
1094-
"description": "Relationship between SeriesNotes and Series",
1094+
"description": "The series that the series note belongs to.",
10951095
"synonyms": []
10961096
},
10971097
{
@@ -1101,7 +1101,7 @@
11011101
"original property": "Series",
11021102
"singular": false,
11031103
"always matches": true,
1104-
"description": "Relationship between and ",
1104+
"description": "The series notes for the series.",
11051105
"synonyms": []
11061106
}
11071107
]

0 commit comments

Comments
 (0)