On Wed, Jul 10, 2024 at 05:08:56PM -0300, Euler Taveira wrote: > Nice improvement. The numbers for a realistic scenario (10k sequences) are
Thanks for taking a look! > You are changing internal representation from char to int64. Is the main goal > to > validate catalog data? What if there is a new sequence data type whose > representation is not an integer? IIRC 0001 was primarily intended to reduce the amount of memory needed for the sorted table. Regarding a new sequence data type, I'm assuming we'll have much bigger fish to fry if we do that (e.g., pg_sequence uses int8 for the values), and I'd hope that adjusting this code wouldn't be too difficult, anyway. > This code path is adding zero byte to the last position of the fixed string. I > suggest that the zero byte is added to the position after the string length. I'm not following why that would be a better approach. strncpy() will add a NUL to the end of the string unless it doesn't fit in the buffer, in which case we'll add our own via "seqtype[sizeof(seqtype) - 1] = '\0'". Furthermore, the compiler can determine the position where the NUL should be placed, whereas placing it at the end of the copied string requires a runtime strlen(). > l = strlen(PQgetvalue(res, 0, 0)); > Assert(l < sizeof(seqtype)); > strncpy(seqtype, PQgetvalue(res, 0, 0), l); > seqtype[l] = '\0'; I think the strncpy() should really be limited to the size of the seqtype buffer. IMHO an Assert is not sufficient. > If you are not planning to apply 0003, make sure you fix collectSequences() to > avoid versions less than 10. Move this part to 0002. Yeah, no need to create the table if we aren't going to use it. > Since you apply a fix for pg_sequence_last_value function, you can simplify > the > query in 0003. CASE is not required. Unfortunately, I think we have to keep this workaround since older minor releases of PostgreSQL don't have the fix. > patched (0001 and 0002): > real 0m0,290s > user 0m0,038s > sys 0m0,104s > > I'm not sure if 0003 is worth. Maybe if you have another table like you > suggested. What pg_dump command did you test here? Did you dump the sequence data, or was this --schema-only? -- nathan
