datafuselabs / databend

𝗗𝗮𝘁𝗮, 𝗔𝗻𝗮𝗹𝘆𝘁𝗶𝗰𝘀 & 𝗔𝗜. Modern alternative to Snowflake. Cost-effective and simple for massive-scale analytics. https://databend.com
https://docs.databend.com
Other
7.7k stars 729 forks source link

Remove redundant fields from plan `Insert`: `database` and `table`. #15513

Closed drmingdrmer closed 4 months ago

drmingdrmer commented 4 months ago

When building an Insert plan:

The table is fetched here:

https://github.com/datafuselabs/databend/blob/a4fb4f7973c3ceacb8f67fcb57c0f1cc9f0d52d6/src/query/sql/src/planner/binder/insert.rs#L93-L96

And TableInfo can be fetch with table.get_table_info(). And TableInof.desc is in form of <database>.<table>. Thus there is no need to keep field database and table in plan Insert, since database and table are only used for display, and can be replace with TableInfo.desc.

https://github.com/datafuselabs/databend/blob/a4fb4f7973c3ceacb8f67fcb57c0f1cc9f0d52d6/src/query/sql/src/planner/binder/insert.rs#L192-L200

And Insert::table_info does not have to be an Option anymore.

guojidan commented 4 months ago

/take

guojidan commented 4 months ago

database and table not only used for display. get_table and validate_table_access and HookOperator::create also need them, so I think we do not need remove them.

guojidan commented 4 months ago

cc @drmingdrmer

drmingdrmer commented 4 months ago

Thank you for investigating this issue. It seems addressing it is not as straightforward as initially anticipated.

I'm gonna spend more time to see why database and table are used more than expected.