Re: [HACKERS] Cube extension point support // GSoC'13

On 09.10.2013 21:07, Robert Haas wrote:
> On Tue, Sep 24, 2013 at 9:07 AM, Stas Kelvich<stas(dot)kelvich(at)gmail(dot)com> wrote:
>> Hello
>>
>> There is new version of patch. I have separated ordering operators to different patch (https://commitfest.postgresql.org/action/patch_view?id=1243), fixed formatting issues and implemented backward compatibility with old-style points in cube_is_point() and cube_out().
>>
>> Also comparing output files I've discovered that this four files is combination of two types of different behavior:
>>
>> 1) SELECT '-1e-700'::cube AS cube;
>> can be (0) or (-0)
>>
>> 2) Amount of zeros in exponent of floating point, i.e. SELECT '1e27'::cube AS cube;
>> can be (1e+027) or (1e+27)
>>
>> On my system (OSX) it is second option in both situations. I've also tested it on FreeBSD 9.0 and Ubuntu 12.04 with the same results. So is there some ideas how can I reproduce such results?
>
> Heikki, are you going to review this further for this CommitFest?

Sorry, I didn't realize the ball was in my court.

I went through the patch now, kibitzing over some minor style issues.
Attached is a new version.

This seems good for commit except for two things:

1. The alternative expected output files still need to be updated. Stas
couldn't find a system where some of those file were used. One option is
to simply commit the patch as is, and see if the buildfarm goes red. If
it doesn't, we can simply remove the alternative files - they are not
used on any supported platform. If some animals go red, we'll get the
required diff from the buildfarm output and apply. So this isn't a
show-stopper.

2. I didn't understand this change:

> @@ -422,24 +439,14 @@ g_cube_union(PG_FUNCTION_ARGS)
> Datum
> g_cube_compress(PG_FUNCTION_ARGS)
> {
> - PG_RETURN_DATUM(PG_GETARG_DATUM(0));
> + GISTENTRY *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
> + PG_RETURN_POINTER(entry);
> }
>
> Datum
> g_cube_decompress(PG_FUNCTION_ARGS)
> {
> GISTENTRY *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
> - NDBOX *key = DatumGetNDBOX(PG_DETOAST_DATUM(entry->key));
> -
> - if (key != DatumGetNDBOX(entry->key))
> - {
> - GISTENTRY *retval = (GISTENTRY *) palloc(sizeof(GISTENTRY));
> -
> - gistentryinit(*retval, PointerGetDatum(key),
> - entry->rel, entry->page,
> - entry->offset, FALSE);
> - PG_RETURN_POINTER(retval);
> - }
> PG_RETURN_POINTER(entry);
> }
>

What did the removed code do, and why isn't it needed anymore?

> Is there a prerequisite patch that hasn't been committed yet?

No.

- Heikki