Side effects in scope management

[nickBes]

There are a couple of places introducing side effect updates of scope which introduces possibility of unknown behavior.

Here are some examples:

sql-lineage/packages/core/src/column-lineage.ts

Lines 302 to 314 in 7412c9f

	visitQuery = (ctx: QueryContext): void => {
	const withCtx = ctx.with();
	if (withCtx) {
	const cteScope: CteScope = { tables: new Map() };
	// `using` guarantees the CTE scope is popped when this if-block exits,
	// even if an exception is thrown during CTE registration or query traversal.
	using _cteScope = this.pushCteScope(cteScope);
	this.registerCtes(withCtx, cteScope);
	this.visit(ctx.queryNoWith());
	} else {
	this.visit(ctx.queryNoWith());
	}
	};

sql-lineage/packages/core/src/column-lineage.ts

Lines 878 to 895 in 7412c9f

	private registerPatternRecognition(ctx: PatternRecognitionContext, scope: QueryScope): void {
	if (!ctx.MATCH_RECOGNIZE()) {
	this.registerAliasedRelation(ctx.aliasedRelation(), scope);
	return;
	}
	const aliasCtx = ctx.identifier();
	const alias = aliasCtx ? getIdentifierText(aliasCtx) : null;
	const columnAliasCtx = ctx.columnAliases();
	const measureCols = columnAliasCtx
	? columnAliasCtx.identifier().map(getIdentifierText)
	: ctx.measureDefinition().map((m) => getIdentifierText(m.identifier()));
	if (alias) {
	scope.tables.set(normalizeId(alias), createScopeTable(alias, measureCols, "derived"));
	}
	}

Preferred behavior would look like

  visitQuery = (ctx: QueryContext): void => {
    const withCtx = ctx.with();
    if (withCtx) {
      // calculate scope
      const cteScope = this.getScopeFromCte(withCtx);
      using _cteScope = this.pushCteScope(cteScope);
      this.visit(withCtx); // then visit
      this.visit(ctx.queryNoWith());
    } else {
      this.visit(ctx.queryNoWith());
    }
  };