Re: [Maria-developers] d260871fccc: MDEV-18967 Load data in system version with long unique does not work

2019-03-20 Thread Sergei Golubchik
Hi, Sachin!

ok to push!

On Mar 20, Sachin Setiya wrote:
> revision-id: d260871fccc (mariadb-10.4.3-87-gd260871fccc)
> parent(s): f6166101e0f
> author: Sachin 
> committer: Sachin 
> timestamp: 2019-03-19 16:43:43 +0530
> message:
> 
> MDEV-18967 Load data in system version with long unique does not work
> 
> Update system versioning fields before generaled columns for left out
> fill_record
> 
Regards,
Sergei
Chief Architect MariaDB
and secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Maria-developers] f6166101e0f: MDEV-18904 Assertion `m_part_spec.start_part >= m_part_spec.end_part' failed in ha_partition::index_read_idx_map

2019-03-20 Thread Sergei Golubchik
Hi, Sachin!

On Mar 20, Sachin Setiya wrote:
> revision-id: f6166101e0f (mariadb-10.4.3-86-gf6166101e0f)
> parent(s): 8076c594ce2
> author: sachinsetia1...@gmail.com 
> committer: Sachin 
> timestamp: 2019-03-19 16:43:43 +0530
> message:
> 
> MDEV-18904 Assertion `m_part_spec.start_part >= m_part_spec.end_part' failed 
> in ha_partition::index_read_idx_map
> 
> We do not support long unique index in partition.

Why not?

I would think that the assert is wrong, there is no reason why
ha_partition::index_read_idx_map() would always hit just one partition.

ha_partition::index_read_map() doesn't have this assert. And if you
check the history of the assert you'll find that the comment originally
was /* How can it be more than one partition with the current use? */

So, apparently now you've introduced a new "use" and there can be more
than one partition, and removing the assert looks justified.

> +CREATE TABLE t1 (a int , unique(a) using hash) PARTITION BY HASH (a) 
> PARTITIONS 2;
> +ERROR HY000: Long unique index is not supported in partition

Also - for the future - there are no "long unique" indexes from the user
point of view. This is only an internal name. The SQL syntax does use
"long unique" concept, documentation doesn't, error messages don't. That
is, users simply won't understand what you mean here.

If we'll need an error here, I'd suggest, for example,

  HASH UNIQUE indexes cannot be used in partitioned tables.

but it's not strictly correct, if some engine would implement hash
indexes natively. The more precise message could be

  HASH UNIQUE indexes cannot be used in partitioned MyISAM tables.

or

  Partitioned MyISAM tables do not support HASH UNIQUE indexes.

(where "MyISAM" is a parameter, of course).

But, again, I am not yet convinced that we need this limitation at all.

Regards,
Sergei
Chief Architect MariaDB
and secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Maria-developers] MDEV-18734 ASAN heap-use-after-free in my_strnxfrm_simple_internal upon update on versioned partitioned table

2019-03-20 Thread Aleksey Midenkov
Hi Sergei!

It turned out that only vcol blobs are affected. They are allocated
by update_virtual_fields(), so it was enough to just refresh their value by
doing update_virtual_fields() again after record is restored from the
queue. Please review the fix:

https://github.com/MariaDB/server/pull/1234


On Tue, Mar 12, 2019 at 1:42 PM Sergei Golubchik  wrote:

> Hi, Aleksey!
>
> On Mar 11, Aleksey Midenkov wrote:
> > Hi, Sergei!
> >
> > ha_partition::handle_ordered_index_scan() stores records in
> > m_ordered_rec_buffer. Then TABLE::update_virtual_fields() updates blob
> > buffer and frees the old one. Then ha_partition::return_top_record()
> > returns record from m_ordered_rec_buffer with stale blob pointer. What
> > should we do with this? I propose to duplicate blob buffer when record
> > gets into m_ordered_rec_buffer.
>
> You can make Field_blob to forget that it owns the buffer.
> And when reading back from the queue, Field_blob will need to take
> over the buffer again.
>
> It's String::release() and String::reset() methods.
>
> The tricky part here is that Field_blob doesn't always own the buffer,
> sometimes the storage engine does. So, when reading from the queue and
> restoring the ownership you'll need to take care to do it only for
> values that Field_blob used to own before, not for all blobs.
>
> Regards,
> Sergei
> Chief Architect MariaDB
> and secur...@mariadb.org
>


-- 
All the best,

Aleksey Midenkov
@midenok
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp