Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL export: inconsistent behaviour while adding DEFINER in dump #88

Closed
fifonik opened this issue Jan 11, 2018 · 7 comments
Labels
Milestone

Comments

@fifonik
Copy link

@fifonik fifonik commented Jan 11, 2018

Expected behavior

Behaviour while adding DEFINER into SQL dumps should be consistent across views, storage procedures and triggers.
In addition, it would be nice to have options (under 'Options' button for example) that allow us to include/exclude definers in dump altogether (not everyone need them all the time, especially when you are moving data from one server to another)

Current behavior

When dumping MySQL database using 'Export database as SQL' tool HeidiSQL dumping DEFINER for VIEWS/STORAGE PROCEDURES, but not dumping them for TRIGGERS.

Steps to reproduce

  1. Select database with triggers, view and storage procedures and dump it.
  2. Check that DEFINER present in dump for VIEWS/STORAGE PROCEDURES, but not for TRIGGERS

I posted it on HeidiSQL forum earlier:
https://www.heidisql.com/forum.php?t=21435#p21950

@fifonik fifonik changed the title SQL export: inconsistent behaviour to adding DEFINER in dump SQL export: inconsistent behaviour while adding DEFINER in dump Jan 12, 2018
@rentalhost rentalhost added the bug label Feb 12, 2018
@rentalhost

This comment has been minimized.

Copy link
Collaborator

@rentalhost rentalhost commented Feb 12, 2018

Confirmed: the Trigger Editor shows the DEFINER in the "CREATE code" tab, but don't dump it from Export dialog. Solution here is probably use the same CREATE source generator from "CREATE code" tab, if possible.

@fifonik

This comment has been minimized.

Copy link
Author

@fifonik fifonik commented Jun 12, 2019

Not sure why in tabletools.pas there is special code used for building trigger's code:

Struc := 'CREATE '+UpperCase(DBObj.ObjType)+' '+Quoter.QuoteIdent(DBObj.Name)+' '+StrucResult.Col('Timing')+' '+StrucResult.Col('Event')+
                ' ON '+Quoter.QuoteIdent(StrucResult.Col('Table'))+' FOR EACH ROW '+StrucResult.Col('Statement');

For func/proc it is just used:
Struc := DBObj.CreateCode;

And looks like the CreateCode is already implemented for triggers (trigger_editor.pas) and is supporting definer.

@fifonik

This comment has been minimized.

Copy link
Author

@fifonik fifonik commented Jun 12, 2019

It is possible to make the following changes in tabletools.pas:

uses main, mysql_structures;

const
+  REGEXP_REMOVE_AUTO_INCREMENT = '\sAUTO_INCREMENT\s*\=\s*\d+\s';
+  REGEXP_REMOVE_DEFINER = '\sDEFINER\s*\=\s*\S+\s';

...

if menuExportRemoveAutoIncrement.Checked then begin
-  rx := TRegExpr.Create;
-  rx.ModifierI := True;
-  rx.Expression := '\sAUTO_INCREMENT\s*\=\s*\d+\s';
-  Struc := rx.Replace(Struc, ' ', false);
-  rx.Free;
+ Struc := Remove(Struc, REGEXP_REMOVE_AUTO_INCREMENT);
end;

...

+function TfrmTableTools.Remove(Struc: String, Regexp: String);
+begin
+  rx := TRegExpr.Create;
+  rx.ModifierI := True;
+  rx.Expression := Regexp;
+  Result := rx.Replace(Struc, ' ', false);
+  rx.Free;
+end;

And then conditionnaly remove definers from functions, procedures, views and triggers (new option should also be added into UI):

lntFunction, lntProcedure: begin
  Struc := DBObj.CreateCode;
+  if menuExportRemoveDefiner.Checked then begin
+    Struc := Remove(Struc, REGEXP_REMOVE_DEFINER);
+  end;

...

lntTrigger: begin
-  StrucResult := DBObj.Connection.GetResults('SHOW TRIGGERS FROM '+DBObj.QuotedDatabase+' WHERE `Trigger`='+esc(DBObj.Name));
-  Struc := 'CREATE '+UpperCase(DBObj.ObjType)+' '+Quoter.QuoteIdent(DBObj.Name)+' '+StrucResult.Col('Timing')+' '+StrucResult.Col('Event')+
      ' ON '+Quoter.QuoteIdent(StrucResult.Col('Table'))+' FOR EACH ROW '+StrucResult.Col('Statement');
+  Struc := DBObj.CreateCode;
+  if menuExportRemoveDefiner.Checked then begin
+    Struc := Remove(Struc, REGEXP_REMOVE_DEFINER);
+  end;

...

lntView: begin
  Struc := Struc + Quoter.QuoteIdent(DBObj.Name);
  Output(Struc, True, True, True, True, True);
  Struc := DBObj.CreateCode;
+  if menuExportRemoveDefiner.Checked then begin
+    Struc := Remove(Struc, REGEXP_REMOVE_DEFINER);
+  end;
  if ToDb then
    Insert(Quoter.QuoteIdent(FinalDbName)+'.', Struc, Pos('VIEW', Struc) + 5 );

P.S. Regexp for removing definer should be double checked. Only the first definer should be replaced as sp/triggers may contains 'DEFINER=xyz' in body that should not be removed.

@pf18387

This comment has been minimized.

Copy link

@pf18387 pf18387 commented Jun 12, 2019

Not sure why in tabletools.pas there is special code used for building trigger's code:

Struc := 'CREATE '+UpperCase(DBObj.ObjType)+' '+Quoter.QuoteIdent(DBObj.Name)+' '+StrucResult.Col('Timing')+' '+StrucResult.Col('Event')+
                ' ON '+Quoter.QuoteIdent(StrucResult.Col('Table'))+' FOR EACH ROW '+StrucResult.Col('Statement');

For func/proc it is just used:
Struc := DBObj.CreateCode;

And looks like the CreateCode is already implemented for triggers (trigger_editor.pas) and is supporting definer.

In my humble opinion, the line at

Struc := 'CREATE '+UpperCase(DBObj.ObjType)+' '+Quoter.QuoteIdent(DBObj.Name)+' '+StrucResult.Col('Timing')+' '+StrucResult.Col('Event')+

could also start with
Struc := 'CREATE DEFINER='+Quoter.QuoteIdent(StrucResult.Col('Definer'),true,'@')+' '+…

@ansgarbecker ansgarbecker added this to the v10.3 milestone Nov 12, 2019
@ansgarbecker

This comment has been minimized.

Copy link
Collaborator

@ansgarbecker ansgarbecker commented Nov 13, 2019

Trigger export was implemented for an old Google Code ticket 334 in e5ae3a9.
I'll give it a try and use DBObj.CreateCode for it instead of creating it manually. I suppose I did that for leaving away the definer, but I'm unsure as I did not leave a relevant comment.
That drop-down menu item "Remove DEFINER clause" also sounds good.

@ansgarbecker

This comment has been minimized.

Copy link
Collaborator

@ansgarbecker ansgarbecker commented Nov 13, 2019

be689ed now adds that "Remove DEFINER clause" to the dropdown, which works for triggers, functions, procedures. In the second commit I recalled also views and events can have a definer.

@fifonik

This comment has been minimized.

Copy link
Author

@fifonik fifonik commented Nov 13, 2019

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.