maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexey!

On Nov 03, Alexey Botchkov wrote:
> 
> > Hmm... So, you basically, put an array of properties in frm, under EXTRA2_GIS
> > Two comments:
> > 1. Why n_field, length and decimals? I thought you only add srid,
> > other properties are already stored elsewhere, aren't they?
> 
> the 'length' - it's the coordinate length, not the field length 
> parameter that is presently stored in the FRM.
> 
> Decimals can't be stored for the GEOMETRY field in ordinary way.
> 
> #define FIELDFLAG_BLOB 1024 // mangled with decimals!
> #define FIELDFLAG_GEOM 2048 // mangled with decimals!

Ok. So, decimals weren't stored anywhere before your patch?
What these decimals are? What the syntax look like?

It would be good if your commit would include some test cases...

> n_field - well that is not really needed. Planned to use it as a link to
> the field initially, but that's lost it's sence.
> 
> >2. Really think about upgrades downgrades. The "extra" section in the frm
> > doesn't handle them well, I know how these problems happen.
> > Think, what happens when you add more properties here?
> > What if you remove some? How new server will parse old frms, how
> > old server will read new frms?
> 
> I thought the way to add new properties is to introduce more
> EXTRA2_GIS_something sections in the 'extra' part. Do you think i'd
> better implement similar sections to be inside the EXTRA2_GIS block?

No, I was just asking.
So, when you add more properties, it'll be EXTRA2_GIS2, etc ?
Okay. Perhaps in that case I'd suggest to rename EXTRA2_GIS to
EXTRA2_GIS_SRID_AND_DECIMALS (or something that explains what's stored
there). I thought that EXTRA2_GIS is a storage for all new and future
gis attributes.

> >> diff --git a/sql/sql_lex.h b/sql/sql_lex.h
> >> --- a/sql/sql_lex.h
> >> +++ b/sql/sql_lex.h
> >> @@ -2358,6 +2358,7 @@ struct LEX: public Query_tables_list
> >>   char *backup_dir; /* For RESTORE/BACKUP */
> >>   char* to_log; /* For PURGE MASTER LOGS TO */
> >>   char* x509_subject,*x509_issuer,*ssl_cipher;
> >> + char *srid;
> >
> >really? it can only be used for field declarations, I don't think there's
> >a need to put it in LEX
... 
> if the SRID is not stored in the Lex how do you think it should be 
> transferred to the add_field_to_list?

Right. This is a problem.

Still, I don't like that everything a parser might ever need is simply
dumped into the LEX structure.

So I've create a patch to remove almost all field attributes from LEX -
the parser now creates Create_field and puts everything in there directly.

It's MDEV-7062, currently in bb-10.1-serg branch waiting for Bar to
review it (because it changes some charset related behavior).

I hope he'll review it first thing next week (or even earlier if he'll
have time from 10.0 bugs) and then you'll be able to use it.

Regards,
Sergei