adriangb commented on PR #14057: URL: https://github.com/apache/datafusion/pull/14057#issuecomment-2620509986
I opened a PR with a lot of docstrings and some minor code edits: https://github.com/chenkovsky/datafusion/pull/1/files @chenkovsky could you take a look and see if it's mergeable? I'm hoping since it's mostly docstrings there shouldn't be much issue. I do have two higher level pieces of feedback on this change: 1. I suggest we rename these to `system columns` as they are called in Postgres, especially because of confusion with metadata of the schema (`DFSchema.inner.schema.metadata`) vs schema of the metadata (`DFSchema.metadata`). 2. Is it really not possible to have a better API than the global `METADATA_OFFSET`? It seems really... terrible honestly. I did not try to implement something else so maybe there's a good reason for it that I am not seeing, but could @chenkovsky or @jayzhan211 give me a quick pitch on why this is necessary before I embarked on a futile path. In my mind we could always append metadata columns to the end right? If you have `a,b` and `meta1,meta2` the indices are `1,2,3,4` respectively. And if you append `c` the metadata columns get bumped. Do we promise that indices are preserved in some way if you modify the schema? Would this break anything? -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
