Hi,

I have also written a test for the alter, show and create functions. I have pushed it, also I am about to go through some tests already written and try to make the tests similar to them.

On Sat, Jun 4, 2016 at 5:34 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

I am done with ALTER FUNCTION, i have done this by adding new field. I have committed the code. Please review. 

Also I wanted to discuss more about the new filed which i added to the sp_chistics struct. 

Also I am writing tests for the alter queries. I would also need some more suggestions about what I should do next . 

On Sat, Jun 4, 2016 at 10:49 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

I do but to update the function, the function prototype of  sp_update_routine() is

                   sp_update_routine(THD *thd, stored_procedure_type type, sp_name *name,st_sp_chistics *chistics), so to pass if the field to be updated is AGGREGATE I have to have some way to send the updated value to the above function. 

On Sat, Jun 4, 2016 at 2:24 AM, Sanja <sanja.byelkin@gmail.com> wrote:

Don't you have it in sp_head?

Am 03.06.2016 22:33 schrieb "Varun Gupta" <varungupta1803@gmail.com>:

Hi,

I need a bit of suggestion on how to pass that we are changing the aggregate field, from the parser I need to send the field so in the mysql_execute_function I can call the sp_update_routine.

My ideas is

1) addition of one field to lex structure for aggregate.

2) instead of having is_aggregate in sp_head, we could put that field in in sp_name.

On Fri, Jun 3, 2016 at 11:41 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi, 

I have added the syntax , patch committed, please review it :) 

On Fri, Jun 3, 2016 at 10:35 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

After going through the code I come to the conclusion that a rule for AGGREGATE field needs to be added to the  ALTER FUNCTION characteristics. So I am going forward with adding this syntax. 

On Fri, Jun 3, 2016 at 6:14 PM, Sanja <sanja.byelkin@gmail.com> wrote:

Hi!

ALTER TABLE usually has the same syntax as CREATE (at least I do not remember adding option like here) so I doubts that it is correct.

On Fri, Jun 3, 2016 at 2:40 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

For the alter function func_name field value,

the syntax we have is 

ALTER FUNCTION_SYM sp_name sp_a_chistics

sp_a_chistics:

          /* Empty */ {}

        | sp_a_chistics sp_chistic {}

sp_a_chistics:

                       | AGGREGSTE_SYM option

option:

                       | YES

                       | NO

or option could be any string.

On Fri, Jun 3, 2016 at 1:22 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

The error is fixed , now show works for aggregate functions too :)

On Fri, Jun 3, 2016 at 1:01 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

For non aggregate function , show output is correct and when I run the query SELECT * from mysql.proc table, aggregate field shows NO.

For aggregate function, when I run the query SELECT * from mysql.proc table, aggregate field shows YES.

On Fri, Jun 3, 2016 at 12:56 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Yes it does, but I am not adding anything to the buffer in that case. 

On Fri, Jun 3, 2016 at 12:53 PM, Sanja <sanja.byelkin@gmail.com> wrote:

Could you be more specific? what test?

Does your code change SHOW for non-aggregate function?

On Fri, Jun 3, 2016 at 9:20 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

To solve the problem with the buffer ,can you tell me is there any tests that are run before I run my own tests .

I meant if I change the value of the buf->append(STRING_WITH_LEN("FUNCTION ")); to buf->append(STRING_WITH_LEN("pUNCTION ")); , even then i get the same error.

On Fri, Jun 3, 2016 at 1:32 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

Have you gone through the patch. Do you like that I have added a field agg_res to the functions ? Or should I find some other to figure it out .

On Thu, Jun 2, 2016 at 10:59 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

patch commited .

On Thu, Jun 2, 2016 at 10:53 PM, Vicențiu Ciorbaru <cvicentiu@gmail.com> wrote:

Strange. Need to try this out myself. If you don't figure it out I'll come back later tonight with some info.

Make sure the lengths are indeed correct. Maybe you are getting a segfault there. Or maybe some code expects that string to have a different length for some reason.

Vicentiu

On Thu, 2 Jun 2016 at 20:21, Varun Gupta <varungupta1803@gmail.com> wrote:

If I just remove the buf->append statement it works .

On Thu, Jun 2, 2016 at 10:48 PM, Vicențiu Ciorbaru <cvicentiu@gmail.com> wrote:

Hi Varun,
Your problem is before this code I think. Does it work without your changes in show_create_sp?

Vicentiu

On Thu, 2 Jun 2016 at 20:16, Varun Gupta <varungupta1803@gmail.com> wrote:

In the file sp.cc, function show_create_sp, we have the buffer that holds the output .

1) First i have the  buf->alloc , here I change the size to also include AGGREGATE.

2) Then i do buf->append(STRING_WITH_LEN("AGGREGATE ")), but this gives an error, 

ERROR : At line 78: query 'call mtr.check_testcase()' failed: 1457: Failed to load routine mtr.check_testcase. The table mysql.proc is missing, corrupt, or contains bad data (internal code -6)

On Thu, Jun 2, 2016 at 10:33 PM, Vicențiu Ciorbaru <cvicentiu@gmail.com> wrote:

Hi varun,
Can you be more specific? Lots of things can go wrong.
Vicentiu

On Thu, 2 Jun 2016 at 20:02, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

for the show_create_routine , i am trying to add the AGGREGATE string to the buffer , but I am getting an error. For the query SHOW CREATE FUNCTION func_name , is there a script where I need to make changes ?

On Thu, Jun 2, 2016 at 4:02 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

I have fixed the errors and now the aggregate field is getting stored in the database. I have made a commit.

Now I am working on the show_create_sp to include aggregate in the output.

On Thu, Jun 2, 2016 at 12:48 PM, Sanja <sanja.byelkin@gmail.com> wrote:

/* DB storage of Stored PROCEDUREs and FUNCTIONs */

enum

{

On Thu, Jun 2, 2016 at 9:16 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

In the second point which place are you talking about

On Thu, Jun 2, 2016 at 12:33 PM, Sanja <sanja.byelkin@gmail.com> wrote:

Above assert means that Aria engine is not initiated (it would be better to have whole stack trace).

1) Primary key is not a field but index :)

2) I do not see that you changed enum where field of the database numbered.

On Thu, Jun 2, 2016 at 6:59 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

I have put the aggregate field just before the primary key field . I hope this is fine . I have pushed the change on github.  

On Thu, Jun 2, 2016 at 12:59 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

This is the error that leads to sigabrt error

mysqld: /home/batman/gsoc/MARIADB/server/storage/maria/ma_create.c:83: maria_create: Assertion `maria_inited' failed.

On Thu, Jun 2, 2016 at 12:06 AM, Sanja <sanja.byelkin@gmail.com> wrote:

mtr has --boot-ddd option or just find other way to say us an error which prevents bootstrap SQL running

Am 01.06.2016 20:33 schrieb "Varun Gupta" <varungupta1803@gmail.com>:

Hi,

I have put the field at the end of the table but still I am not able to install system databases. 

Could not install system database from /home/batman/gsoc/MARIADB/server/mysql-test/var/tmp/bootstrap.sql . I have seen this file looks fine to me .

On Wed, Jun 1, 2016 at 11:04 PM, Vicențiu Ciorbaru <vicentiu@mariadb.org> wrote:

Hi Varun,

Adding extra fields to the proc table should be done at the end of the table. We have code that assumes the table has columns in that particular order. This is most likely why you are getting the failures.

Regards,

Vicentiu

On Wed, 1 Jun 2016 at 20:05 Sanja <sanja.byelkin@gmail.com> wrote:

1) Why you decided to put it in the middle? for more incompatibility?

2) Do you know that space should be put after come and not before? (it is about SQL you changed).

3) check that there is no really mtr.check_testcase if it is absent then check how your changes broke creation of procedures (SQL of creation the procedure you can find in mysql-test/include/mtr_check.sql

If it is there then check why code of finding procedure become broken.

On Wed, Jun 1, 2016 at 6:13 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hey,

I have made changes to the code and have pushed the code on github, I am stuck with an error which I can't figure out . The error is 

 At line 78: query 'call mtr.check_testcase()' failed: 1305: PROCEDURE mtr.check_testcase does not exist

not ok

On Wed, Jun 1, 2016 at 3:40 PM, Sanja <sanja.byelkin@gmail.com> wrote:

Yes, called from mysql_install_db

On Wed, Jun 1, 2016 at 11:59 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

Sanja , in mysql_system_tables.sql is where I found tables are created? Are u talking about this script.

On Wed, Jun 1, 2016 at 3:17 PM, Sanja <sanja.byelkin@gmail.com> wrote:

Probably you have to change code where the table is created (if I remember correctly it is bootstrap script), then recreate database (mysql-test-run is doing it for you)

then upgrade script should be fixed but it can be done later.

On Wed, Jun 1, 2016 at 11:44 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

Well I am getting an error which says that column expected 21 but are 20. Do I need to drop the already created mysql.proc table? In my opinion I should.

On Wed, Jun 1, 2016 at 1:23 PM, Vicențiu Ciorbaru <vicentiu@mariadb.org> wrote:

Hi Varun,

Looks good to me.

Vicentiu

On Wed, 1 Jun 2016 at 10:49 Varun Gupta <varungupta1803@gmail.com> wrote:

Hi,

As I was going through with adding a field to the proc table.

This is what I think  should be done, please correct me if I am wrong :)

First add a new field to the enum { MYSQL_PROC_FIELD_IS_AGGREGATE }

Then store the value in table->field[MYSQL_PROC_FIELD_IS_AGGREGATE]->store(sp->is_aggregate ?1:2),TRUE);

On Wed, Jun 1, 2016 at 9:32 AM, Varun Gupta <varungupta1803@gmail.com> wrote:

Hi Vicentiu,

As Sanja was telling  that the mysql.proc table should have an additional field  to store if a function is AGGREGATE or not. Well I completely agree with it. So I can add that or by the FETCH GROUP NEXT ROW , we can tell a function is aggregate or not , in such case we would not need any new field in the mysql.proc table

On Tue, May 31, 2016 at 9:46 PM, Sanja <sanja.byelkin@gmail.com> wrote:

ok

On Tue, May 31, 2016 at 6:12 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

For the aggregate functions like for sum, we go through these functions as in these function we have the aggregator which calls the add() function

On Tue, May 31, 2016 at 8:52 PM, Sanja <sanja.byelkin@gmail.com> wrote:

Hi!

On Tue, May 31, 2016 at 5:10 PM, Varun Gupta <varungupta1803@gmail.com> wrote:

For the aggregate results as we have functions which we need to execute from sp_head::execute  

a) For the first record , we use init_sum_functions

b) For the other records we would have update_sum_functions

We would have the entire aggregation in these 2 functions.

Why? I do not understand why we need this and what they will do.

Maybe, you do not understand how it works now.

Item_field has reference to the table  and as soon as table set to the correct record it will read correct values

But it should not bother you! You get expressions from the parameters and get values from them, the question is only that it should be done in correct moment when tables set on the record you need (and actually you should not even know what is under the hood because it can be even different table if aggregation made via temporary table.

it is completely legal to ask SUM(a-b) where a and b could be fields of different tables.