Re: Compressed TOAST Slicing
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: tested, passed Spec compliant: not tested Documentation:not tested I have read the patch and have no problems with it. The feature is super valuable for complex PostGIS-enabled databases.
Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested This patch is particularly helpful in processing OpenStreetMap Data in PostGIS. OpenStreetMap is imported as a stream of 300-900 (depending on settings) gigabytes, that are needing a VACUUM after a COPY FREEZE. With this patch, the first and usually the last transforming query is performed much faster after initial load. I have read this patch and have no outstanding comments on it. Pavan Deolasee demonstrates the expected speed improvement. The new status of this patch is: Ready for Committer
Re: New vacuum option to do only freezing
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested I have read this patch. I like the concept and would like it to get committed. Question I have after reading the patch is around this construct: /* -* If there are no indexes then we can vacuum the page right now -* instead of doing a second scan. +* If there are no indexes we can vacuum the page right now instead of +* doing a second scan. Also we don't do that but forget dead tuples +* when index cleanup is disabled. */ This seems to change behavior on heap tuples, even though the option itself is documented to be about "Indexes" only. This needs either better explanation what "forget dead tuples" means and that it does not lead to some kind of internal inconsistency, or documentation on what is the effect on heap tuples. This same block raises a question on "after I enable this option, do a vacuum, decide I don't like it, what do I need to do to disable it back?" - just set it back, or set and perform a vacuum, or set and perform a VACUUM FULL as something was "forgotten"? It may happen this concept of "forgetting" is documented somewhere in the near comments but I'd prefer it to be stated explicitly. The new status of this patch is: Waiting on Author
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested I have read disable-vacuum-truncation_v6.patch. I like it the way it is. Word "truncation" for me means "remove everything from the table" (as in TRUNCATE) and I don't want VACUUM to do that :), shrink feels appropriate. TRIM has a connotation from SSD drives, where you can discard blocks in the middle of your data and let the underlying infrastructure handle space allocation on it. Please keep it for the good times Postgres can punch a hole in the middle of relation and let filesystem handle that space. The new status of this patch is: Ready for Committer
Re: Cast jsonb to numeric, int, float, bool
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested We're using this patch and like it a lot. We store a lot of log-like data in s3-hosted .json.gz files. Sometimes we need to suddenly ingest them and calculate statistics and check the new what-ifs. We ingest data to simple single-column table with jsonb field, and then perform our calculation on top of it. Without this patch the only option to get data already parsed as numbers into jsonb into calculation is via binary-text-binary transformation. We're relying on the patch for our custom spatial type extension and casting in it. https://github.com/gojuno/lostgis/blob/master/sql/types/type_tpv.sql#L21 For postgres installations without the patch we do WITH INOUT cast stubbing, https://github.com/gojuno/lostgis/blob/master/sql/types/__jsonb_casts.sql - but without performance benefits of raw C processing :) A thing this patch does not implement is cast from jsonb to bigint. That would be useful for transforming stored osm_id OpenStreetMap object identifiers. Right now we're stubbing it with jsonb::numeric::bigint cast, but the middle one would be nice to get rid of. The new status of this patch is: Ready for Committer
Re: [PATCH][PROPOSAL] Add enum releation option type
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested I believe this patch simplifies infrastructure. As non native speaker, I'm okay with presented version of comma.
Re: compress method for spgist - 2
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:tested, passed I've read the updated patch and see my concerns addressed. I'm looking forward to SP-GiST compress method support, as it will allow usage of SP-GiST index infrastructure for PostGIS. The new status of this patch is: Ready for Committer
