[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

[rednaxelafx <gi...@git.apache.org>]

Github user rednaxelafx commented on the issue:

    https://github.com/apache/spark/pull/20419
  
    I gave it a bit more thought. Here's an alternative proposal: instead of using a "force comment" mechanism in the current form (which still gets a `ctx.freshName("c")` that the caller has no control over), why don't we provide an alternative API that also forces comment, but allows the caller to specify a (within the `CodegenContext`) unique ID for the placeholder comment?
    
    In this case, instead of using `/*wholestagecodegen_c*/` as the placeholder, which can be brittle if someone adds new calls to `ctx.registerComment()` in `WholeStageCodegenExec`, we can call `ctx.registerComment(comment, placeholderId = "wsc_codegenStageId")`, which we can be sure that it'll end up creating a placeholder comment of `/*wsc_codegenStageId*/`, stable across all whole-stage generated main classes, that would never affect the codegen cache behavior.
    
    @kiszk WDYT?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org