LucaCappelletti94 opened a new pull request, #2592: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2592
As @xitep pointed out back in #2218 the `Statement` enum is very large, back then about 2KB, and now has surpassed 3KB on main because `CreateTable` and the other large payloads are stored inline. Every slot of the `Vec<Statement>` the parser returns pays that size, and so does every query body, since `SetExpr` holds a `Statement` inline for `Insert`, `Update`, `Delete` and `Merge` and each `Box<SetExpr>` therefore allocates 3440 bytes, `SELECT 1` included. Parsing a script of 12K short statements retains 109 MB. This PR puts 139 of the 140 data-carrying variants behind a `Box`. `Statement` drops to 24 bytes and `SetExpr` to 32, the `large_enum_variant` allow is gone and a test keeps `Statement` at 24 bytes or less. **Retained memory falls 54% on the 12K statements script parse time on statement-heavy scripts drops ~10%, and single-statement parses are about 7% faster.** Now, I understand that reviewing this as a +5k/-5k line is nearly impossible, so if you agree that it is actually desirable to proceed with this non-trivial refactoring which will definitely break code downstream (I checked that in DataFusion 23 of 35 sites need an update, all in `datafusion/sql`), I will proceed to turn it into a very long stacked PR which can then be processed a statement at the time. If you reckon that it is an excessive API break and the gains are insufficient, I will close the PR. *Refactoring drafted with Opus 5.5* cc @iffyio @alamb -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
