Re: [Maria-developers] InnoDB blob for primary key
Hello Sergei!
Sir please review the prototype at
https://github.com/SachinSetiya/server/tree/unique_index_sachin
this is half code I guess it will be complete by tomorrow
Regards
sachin
On Mon, May 2, 2016 at 8:42 PM, Sergei Golubchik wrote:
> Hi, Sachin!
>
> On May 02, Sachin Setia wrote:
> > Hi Sergei!
> >
> > As i told you i was prototyping for hash table
> > It is done around 90% apart from one thing when hash is same
> > how to get record from .myd file when i have offset of record
> > so currently i am skipping it
> > But it is very fast i do not know why this is so fast here are
> > results of employee database
> > salary table definition
> > CREATE TABLE salaries (
> > emp_no INT NOT NULL,
> > salary blob NOT NULL,
> > from_date DATENOT NULL,
> > to_date DATENOT NULL,
> > FOREIGN KEY (emp_no) REFERENCES employees (emp_no) ON DELETE CASCADE,
> > PRIMARY KEY (emp_no, from_date)
> > )
>
> I presume, you had ENGINE=Aria here? Because your code patch is for Aria.
>
> > ;
> > And query is
> > MariaDB [employees]> select distinct salary from salaries;
> >
> > Result with out using hash table
> >
> > ++
> > 85814 rows in set (2 min 24.76 sec)
> >
> >
> > Result with using hash table
> >
> > | 39420 |
> > ++
> > 85809 rows in set (6.24 sec)
> >
> > ( number of rows are not equal but this can be solved if i get record by
> > offset)
> >
> > I am sure there is something wrong.The whole hash table is in memory
> > like wise the b tree of hash is in memory but why there is so much
> > improvement. Please sir check the prototype and tell if i am wrong
> > .thanks
>
> Sure.
>
> But still, please put your code on github and commit often.
> Let's use a proper development process instead of sending patches back
> and forth. If you need help with that, feel free to ping me on irc.
>
> And using // comments makes the code more difficult to review - you
> change every line in a big block. Better use
>
> #if 0
>...
> #endif
>
> > diff --git a/storage/maria/ma_hash_table.h
> b/storage/maria/ma_hash_table.h
> > new file mode 100644
> > index 000..c8e4578
> > --- /dev/null
> > +++ b/storage/maria/ma_hash_table.h
>
> why are you doing it in Aria? I thought we've agreed to do it on the
> upper level, in sql/
>
> > @@ -0,0 +1,45 @@
> > +#include"../../mysys/my_malloc.c"
> > +#include"../../include/my_global.h"
> > +typedef struct ma_hash_table_element{
> > + unsigned int hash_code;
> > + unsigned int record_offset;
> > + struct ma_hash_table * next; //we will use single link list
> because no delete operations
> > +} ma_hash_table_element;
>
> Did you look at reusing the HASH data structure? include/hash.h,
> mysys/hash.c?
>
> > +
> > +typedef struct ma_hash_table{
> > + unsigned int size;
> > + ma_hash_table_element * h_t_e;
> > +}ma_hash_table;
>
> Because of the above (on the wrong level, ma_hash_table instead of HASH,
> using // comments to disable code) I've only quickly looked through the
> patch.
> But I didn't notice anything obviously wrong.
>
> 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-10035 DBUG_ASSERT on CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE
On 16.05.2016 16:05, Sergei Golubchik wrote: Hi, Oleksandr! On May 16, Oleksandr Byelkin wrote: That's different. See: CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE; SELECT * FROM v1, t2; This is different from SELECT * FROM t1, t2 FOR UPDATE; the first locks only t1, the second - both t1 and t2. meant, what the user expectations could be. Yes, but they are against at least standard, and we should think a lot before implement yet another non-standard feature. Sure. FOR UPDATE, LOCK IN SHARE MODE, and clauses I've mentioned in another email are all non-standard. The question is not about implementing another non-standard feature, but about how the combination of existing features should work. This is a tradeoff between the principle of the least surprise, how much effort we want to put into this, and whether there's anyone who expects it to work :) OK, but you have not answered question about priority, because both has sense (at least for me). ___ 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-10035 DBUG_ASSERT on CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE
Hi, Oleksandr! On May 16, Oleksandr Byelkin wrote: > >>> That's different. See: > >>> > >>> CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE; > >>> SELECT * FROM v1, t2; > >>> > >>> This is different from > >>> > >>> SELECT * FROM t1, t2 FOR UPDATE; > >>> > >>> the first locks only t1, the second - both t1 and t2. > > meant, what the user expectations could be. > > Yes, but they are against at least standard, and we should think a lot > before implement yet another non-standard feature. Sure. FOR UPDATE, LOCK IN SHARE MODE, and clauses I've mentioned in another email are all non-standard. The question is not about implementing another non-standard feature, but about how the combination of existing features should work. This is a tradeoff between the principle of the least surprise, how much effort we want to put into this, and whether there's anyone who expects it to work :) 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-10035 DBUG_ASSERT on CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE
On 16.05.2016 14:56, Sergei Golubchik wrote: Hi, Oleksandr! On May 16, Oleksandr Byelkin wrote: On May 16, Alexander Barkov wrote: Just now this parameter for VIEWs effectively (and silently) ignored. So error IMHO is better. It will break existing application where users have FOR UPDATE in the view definition. But, probably, there won't be many? I think there won't be many. For me it looks like when we added FOR UPDATE, we just forgot to disallow this in VIEW. A more clear way is to use FOR UPDATE when invoking views than when creating views: SELECT * FROM view1 FOR UPDATE; That's different. See: CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE; SELECT * FROM v1, t2; This is different from SELECT * FROM t1, t2 FOR UPDATE; the first locks only t1, the second - both t1 and t2. No, "FOR UPDATE" has no any traces in view.frm Yes, it doesn't work now. Above I meant, what the user expectations could be. Yes, but they are against at least standard, and we should think a lot before implement yet another non-standard feature. The thing is theoretically doable, but require changes in current algorithm of processing. Also the question which setup overwrite which. Whole query derived table or vice versa. ___ 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-10035 DBUG_ASSERT on CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE
Hi, Oleksandr! On May 16, Oleksandr Byelkin wrote: > > > > On May 16, Alexander Barkov wrote: > Just now this parameter for VIEWs effectively (and silently) ignored. So > error IMHO is better. > >>> It will break existing application where users have FOR UPDATE in the > >>> view definition. But, probably, there won't be many? > >> I think there won't be many. > >> For me it looks like when we added FOR UPDATE, > >> we just forgot to disallow this in VIEW. > >> > >> A more clear way is to use FOR UPDATE when invoking views than when > >> creating views: > >> SELECT * FROM view1 FOR UPDATE; > > That's different. See: > > > >CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE; > >SELECT * FROM v1, t2; > > > > This is different from > > > >SELECT * FROM t1, t2 FOR UPDATE; > > > > the first locks only t1, the second - both t1 and t2. > No, "FOR UPDATE" has no any traces in view.frm Yes, it doesn't work now. Above I meant, what the user expectations could be. 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-10035 DBUG_ASSERT on CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE
Hi! On 16.05.2016 14:26, Sergei Golubchik wrote: Hi, Alexander! On May 16, Alexander Barkov wrote: Just now this parameter for VIEWs effectively (and silently) ignored. So error IMHO is better. It will break existing application where users have FOR UPDATE in the view definition. But, probably, there won't be many? I think there won't be many. For me it looks like when we added FOR UPDATE, we just forgot to disallow this in VIEW. A more clear way is to use FOR UPDATE when invoking views than when creating views: SELECT * FROM view1 FOR UPDATE; That's different. See: CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE; SELECT * FROM v1, t2; This is different from SELECT * FROM t1, t2 FOR UPDATE; the first locks only t1, the second - both t1 and t2. No, "FOR UPDATE" has no any traces in view.frm (because it implemented by writing properties to individual tables during parsing, which I think is bad thing in all senses). So if it is processed now differently it is only because FOR UPDATE works incorrectly and do not goes inside views (I did not checked it but it is probable). [skip] ___ 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-10035 DBUG_ASSERT on CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE
Hi, Alexander! On May 16, Alexander Barkov wrote: > > > >> Just now this parameter for VIEWs effectively (and silently) ignored. So > >> error IMHO is better. > > > > It will break existing application where users have FOR UPDATE in the > > view definition. But, probably, there won't be many? > > I think there won't be many. > For me it looks like when we added FOR UPDATE, > we just forgot to disallow this in VIEW. > > A more clear way is to use FOR UPDATE when invoking views than when > creating views: > SELECT * FROM view1 FOR UPDATE; That's different. See: CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE; SELECT * FROM v1, t2; This is different from SELECT * FROM t1, t2 FOR UPDATE; the first locks only t1, the second - both t1 and t2. > And by the way, it does not work even with TEMPTABLE. Here is an example: Sure, I didn't say it does. I said it might work with TEMPTABLE and it cannot possibly work with MERGE. Either we fix it to work or we disallow it. > > But also this patch goes directly against the work that Bar is doing > > now. This patch adds new manual syntax checks into the code, while > > Bar is trying to have the grammar to allow only syntaxically corect > > queries. > > I even earlier made a patch disallowing this syntactically. > It was in the previous letter. I'm attaching it again. > > I made it before you commented in MDEV-10063 :) > But then would not sent for review, because we had a discussing > that it might be useful with TEMPTABLE... > > But now when we know that TEMPTABLE does not really work... > Ok to push the attached patch? I think we need to apply the same logic to all per-SELECT clauses. LOCK IN SHARE MODE, FOR UPDATE, HIGH_PRIORITY, STRAIGHT_JOIN, SQL_SMALL_RESULT, SQL_BIG_RESULT, SQL_BUFFER_RESULT. 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-10035 DBUG_ASSERT on CREATE VIEW v1 AS SELECT * FROM t1 FOR UPDATE
Hi Sergei, On 05/16/2016 10:35 AM, Sergei Golubchik wrote: Hi, Oleksandr! On May 16, Oleksandr Byelkin wrote: On 16.05.2016 06:00, Alexander Barkov wrote: Hi Sanja, Sergei, Sanja, you asked me to review a patch for MDEV-10035: revision-id: 4ffe2295e78538dde93df078421726f0c5a7d2a2 (mariadb-10.2.0-29-g4ffe229) parent(s): b79944950e5e5db40cf7ad49061edf5f105512c4 committer: Oleksandr Byelkin timestamp: 2016-05-15 15:25:33 +0200 message: I earlier also proposed the same idea to disallow FOR UPDATE, and even created a patch disallowing this in the parser syntactically (see attached). But Sergei worried that we should not do it this way and proposed some other solutions. Please see Sergei's comments in: MDEV-10063 VIEWs and subqueries with FOR UPDATE Sergei, are you still in doubts? Yes :) Out of all approaches, I think, I'd prefer to issue a warning like "Clause FOR UPDATE" won't work unless you specify TEMPTABLE algorithm. Making it an error in the strict mode, as usual. Just now this parameter for VIEWs effectively (and silently) ignored. So error IMHO is better. It will break existing application where users have FOR UPDATE in the view definition. But, probably, there won't be many? I think there won't be many. For me it looks like when we added FOR UPDATE, we just forgot to disallow this in VIEW. A more clear way is to use FOR UPDATE when invoking views than when creating views: SELECT * FROM view1 FOR UPDATE; And by the way, it does not work even with TEMPTABLE. Here is an example: # Connection#1: DROP TABLE IF EXISTS t1,t2; DROP VIEW IF EXISTS v1; CREATE TABLE t1 (a INT); CREATE TABLE t2 (a INT); INSERT INTO t1 VALUES (10); INSERT INTO t2 VALUES (10); CREATE ALGORITHM=TEMPTABLE VIEW v1 AS SELECT * FROM t1 UNION ALL (SELECT * FROM t2 FOR UPDATE); START TRANSACTION; SELECT * FROM v1 FOR UPDATE; # Connection#2 (does not lock, returns results immediately): START TRANSACTION; SELECT * FROM v1 FOR UPDATE; COMMIT; +--+ | a| +--+ | 10 | | 10 | +--+ # Connection#3 (does not lock either, returns results immediately): START TRANSACTION; SELECT * FROM t2 FOR UPDATE; COMMIT; +--+ | a| +--+ | 10 | +--+ But also this patch goes directly against the work that Bar is doing now. This patch adds new manual syntax checks into the code, while Bar is trying to have the grammar to allow only syntaxically corect queries. I even earlier made a patch disallowing this syntactically. It was in the previous letter. I'm attaching it again. I made it before you commented in MDEV-10063 :) But then would not sent for review, because we had a discussing that it might be useful with TEMPTABLE... But now when we know that TEMPTABLE does not really work... Ok to push the attached patch? Thanks! Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result index c56597e..447afe3 100644 --- a/mysql-test/r/sp-error.result +++ b/mysql-test/r/sp-error.result @@ -1227,14 +1227,14 @@ DROP PROCEDURE IF EXISTS bug14702; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (i INT); CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO @a; -ERROR HY000: View's SELECT contains a 'INTO' clause +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'INTO @a' at line 1 CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO DUMPFILE "file"; -ERROR HY000: View's SELECT contains a 'INTO' clause +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'INTO DUMPFILE "file"' at line 1 CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO OUTFILE "file"; -ERROR HY000: View's SELECT contains a 'INTO' clause +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'INTO OUTFILE "file"' at line 1 CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT i FROM t1 PROCEDURE ANALYSE(); -ERROR HY000: View's SELECT contains a 'PROCEDURE' clause +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'PROCEDURE ANALYSE()' at line 2 CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 FROM (SELECT 1) AS d1; ERROR HY000: View's SELECT contains a subquery in the FROM clause CREATE PROCEDURE bug20953(i INT) CREATE VIEW v AS SELECT i; diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 3fccd6e..47a29a4 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -923,12 +923,12 @@ select * from v4; ERROR 21000: Subquery returns more than 1 row drop view v4, v3, v2, v1; create view v1 as select 5 into @w; -ERROR HY000: View's SELECT contains a 'INTO' clause +ERROR 42000: You have an error in your SQL syn
