AnishMahto commented on code in PR #56686:
URL: https://github.com/apache/spark/pull/56686#discussion_r3521265944
##########
sql/pipelines/src/main/scala/org/apache/spark/sql/pipelines/graph/DatasetManager.scala:
##########
@@ -104,12 +106,38 @@ object DatasetManager extends Logging {
transformer.transformTables { table =>
if (tablesToMaterialize.keySet.contains(table.identifier)) {
try {
- materializeTable(
+ val isFullRefresh =
tablesToMaterialize(table.identifier).isFullRefresh
+ val (tableWithMaterializationMetadata, catalogTableEntity) =
materializeTable(
resolvedDataflowGraph = resolvedDataflowGraph,
table = table,
- isFullRefresh =
tablesToMaterialize(table.identifier).isFullRefresh,
+ isFullRefresh = isFullRefresh,
context = context
)
+ // Auxiliary tables' lifecycle should follow the table that it
is complimentary to.
+ // If this table has any auxiliary tables, validate the target
can host them and
+ // materialize/full-refresh them accordingly.
+
resolvedDataflowGraph.auxiliaryTableSpecs.get(table.identifier).foreach {
Review Comment:
Yeah I'd say it's intentional/acceptable, arguably more correct. A couple
reasons:
- I think the philosophy should be only tables explicitly declared (or in
the aux table's case, derived) in the pipeline definition for the current run
should be touched by the current run. This goes for regular tables already,
where if a table is removed from the pipeline in a subsequent run, the pipeline
doesn't go out of its way to delete it.
- It's a little bit more of a grey area for "internal" tables, but
pipeline-independent persistence could still be useful in case someone
accidentally drops/changes the AutoCDC flow in their pipeline definition - the
next repaired run can directly pick up the still-persisted aux table. On one
hand table cleanup is not "managed", on the other hand users have full control
over the table's lifecycle outside of a pipeline.
If we eventually do want to support a true "cleanup all tables that were
once created by a pipeline, but no longer referenced by a pipeline" then we can
probably maintain a name-based pointer to the aux table in the target table's
properties.
But for now I think this is the right direction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
