Re: bytea_ops

> Looks pretty good.  I have only three gripes, two trivial.  In order of
> decreasing trivialness:
>
> +bytealt(PG_FUNCTION_ARGS)
> +{
> + bytea *arg1 = PG_GETARG_BYTEA_P(0);
> + VarChar *arg2 = PG_GETARG_BYTEA_P(1);
>
> Looks like you missed one VarChar -> bytea.

Ouch! You are of course correct.

>
> + if ((cmp < 0) || ((cmp == 0) && (len1 < len2)))
> + PG_RETURN_BOOL(TRUE);
> + else
> + PG_RETURN_BOOL(FALSE);
>
> I think it's better style to code stuff like this as
>
> PG_RETURN_BOOL((cmp < 0) || ((cmp == 0) && (len1 < len2)));

OK

>
> BoolGetDatum already does the work of ensuring that the returned Datum
> is exactly TRUE or FALSE, so why do it over again?
>
> The biggie is that you missed adding support for bytea to scalarltsel,
> which puts a severe crimp on the optimizer's ability to make any
> intelligent decisions about using your index.  See
> src/backend/utils/adt/selfuncs.c, about line 580.  You need to add a
> case to the convert_to_scalar() function in that file.  I'd say that
> bytea should be a separate type category --- don't try to cram it into
> convert_to_scalar's string category, because the string-handling code
> assumes null-termination is safe.  But the implementation should be
> modeled on the handling of strings: you want to strip any common prefix,
> then use the next few bytes as base-256 digits to form numeric values.
> The comments for convert_string_to_scalar should give you an idea.

I thought I must have missed something here. I noticed that with ~40000 rows
inserted (and the table vacuumed), explain on a < or > query showed the
optimizer tended to not use the index unless severely restricted, i.e.
    select * from foobar where f1 < '\\000\\015';
would do a table scan while
    select * from foobar where f1 < '\\000\\011';
would use the index.

Hopefully fixing the above will improve this. Thank you for looking at this
so quickly :-)

I'll take another pass and send in a new patch soon.

-- Joe