Hi Jian, On Thu, Oct 17, 2024 at 5:59 AM jian he <jian.universal...@gmail.com> wrote: > in [3] I mentioned adding "ParseLoc location" to ExplainStmt, then you > found some problems at [4] with multi statements, > now I found a way to resolve it. > I also add "ParseLoc location;" to typedef struct CopyStmt. > copy (select 1) to stdout; > I tested my change can tracking > beginning location and length of the nested query ("select 1") > > I didn't touch other nested queries cases yet, so far only ExplainStmt > and CopyStmt1 > IMHO, it's more neat than your patches. > Can you give it a try?
I'm not sure about this approach. It moves the responsibility of tracking the location and length from the nested statement to the top level statement. - The location you added in ExplainStmt and CopyStmt has a different meaning from all others and tracks the nested location and not the location of the statement itself. This creates some inconsistencies. - The work will need to be done for all statements with nested queries: Prepare, Create table as, Declare Cursor, Materialised View. Whereas by tracking the location of PreparableStatements, there's no need for additional logic. For example, v8 0002 fixes the existing behaviour for Prepare statements thanks to SelectStmt's modifications. I feel like while it looks simpler, it will need more work to handle all cases while introducing an outlier in how locations are tracked.