From: | "Joe Conway" <joseph(dot)conway(at)home(dot)com> |
---|---|
To: | "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: bytea_ops |
Date: | 2001-08-12 01:58:40 |
Message-ID: | 015a01c122d2$4f554770$0705a8c0@jecw2k1 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
> 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
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2001-08-12 02:06:57 | Re: Re: copy to/from stdout using libpgtcl |
Previous Message | Bruce Momjian | 2001-08-12 01:58:35 | Re: Re: [PATCHES] Select parser at runtime |