phppgadmin-devel — For Developers Only

[ppa-dev] PHP 7

[Karl O. P. <ko...@me...>]

Hi Rob,

I hate to be this way, but so long as you're in
making commits then PHP 7 support would be nice.

This pretty much means changing the regexp calls
to use pcre.  IIRC.

I probably have a patch somewhere if you're interested.

Regards,

Karl <ko...@me...>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

[ppa-dev] Better spanish translation file

[Miguel U. <mig...@sk...>]

Hello,

I've updated Spanish translation, you can see more details at Github's
pull request https://github.com/phppgadmin/phppgadmin/pull/25


-- 
Miguel Ãngel Useche Castro
mig...@sk...
@skatox
Visita http://www.skatox.com/  Blog de Informática, Desarrollo Web,
Mozilla, Software Libre, Linux, Videojuegos y mas....

[ppa-dev] Better spanish translation file

[Miguel U. <mig...@sk...>]

Hello,

I've updated Spanish translation, you can see more details at Github's
pull request https://github.com/phppgadmin/phppgadmin/pull/25


-- 
Miguel Ángel Useche Castro
mig...@sk...
@skatox
Visita http://www.skatox.com/  Blog de Informática, Desarrollo Web,
Mozilla, Software Libre, Linux, Videojuegos y mas....

Re: [ppa-dev] patch for supporting functions with output parameters

[Robert T. <ro...@xz...>]

merged, thanks!

Robert Treat

On Tue, Mar 4, 2014 at 5:53 PM, Szabolcs Hubai <sz...@gm...> wrote:
> Hi all!
>
>
> 2014-02-22 20:07 GMT+01:00 Robert Treat <ro...@xz...>:
>>
>> [...]
>>
>> Feel free to check out from my repo if you want to test it. It'd be
>> nice to fix up the named parameters on OUT/returns stuff, but
>> otherwise I think it's pretty much ready (although if anyone has pre
>> 9.3 versions of postgres handy for testing, it'd be nice to verify
>> this works there as well)
>
> I tested it with the dup function [1] from the docs.
> It was displayed fine with 9.1+134wheezy4 aka 9.1.12.
>
> I see this improvement was landed on phpgadmin/master,
> so it's late to note that the commit introduced some space indented line.
> See my branch [2] for a trivial fix.
>
>
>
> [1]: http://www.postgresql.org/docs/9.3/static/sql-createfunction.html#SQL-CREATEFUNCTION-EXAMPLES
> [2]: https://github.com/xabolcs/phppgadmin/compare/branch-bug-outparams-improvements-revert-email-readability
>
> --
> Szabolcs
>
> ------------------------------------------------------------------------------
> Subversion Kills Productivity. Get off Subversion & Make the Move to Perforce.
> With Perforce, you get hassle-free workflows. Merge that actually works.
> Faster operations. Version large binaries.  Built-in WAN optimization and the
> freedom to use Git, Perforce or both. Make the move to Perforce.
> http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk
> _______________________________________________
> phpPgAdmin-devel mailing list
> php...@li...
> https://lists.sourceforge.net/lists/listinfo/phppgadmin-devel

Re: [ppa-dev] patch for supporting functions with output parameters

[Szabolcs H. <sz...@gm...>]

Hi all!


2014-02-22 20:07 GMT+01:00 Robert Treat <ro...@xz...>:
>
> [...]
>
> Feel free to check out from my repo if you want to test it. It'd be
> nice to fix up the named parameters on OUT/returns stuff, but
> otherwise I think it's pretty much ready (although if anyone has pre
> 9.3 versions of postgres handy for testing, it'd be nice to verify
> this works there as well)

I tested it with the dup function [1] from the docs.
It was displayed fine with 9.1+134wheezy4 aka 9.1.12.

I see this improvement was landed on phpgadmin/master,
so it's late to note that the commit introduced some space indented line.
See my branch [2] for a trivial fix.



[1]: http://www.postgresql.org/docs/9.3/static/sql-createfunction.html#SQL-CREATEFUNCTION-EXAMPLES
[2]: https://github.com/xabolcs/phppgadmin/compare/branch-bug-outparams-improvements-revert-email-readability

--
Szabolcs

Re: [ppa-dev] patch for supporting functions with output parameters

[Robert T. <ro...@xz...>]

On Mon, Feb 24, 2014 at 7:57 AM, Bert Thomas <bt...@br...> wrote:
>> this is a little weird, because afaict OUT parameters never get
>> display (they are just dumped to the returns box), although INOUT are.
>
> I don't understand you. It is not dumped to the returns box, but in the
> "Arguments" box.
>

I'm saying that, I would expect OUT parameters to show up in the
"RETURNS" section of the display, since that's what the function
returns. Maybe that isn't correct though?  Would you expect RETURNS to
be blank, and everything in arguments?

On a side note, any chance you have an example function you could send
for testing; It'd be good to verify we're looking at functions created
the same way.

>
>> This make me wonder what we would see with v or t style functions
>> (didn't have any handy to test on, do you have any?) I think this
>> could probably use more work to make RETURNS section display OUT with
>> param name. (Not a show stopper, but you should look into it).
>
> I cannot think of a reason why it wouldn't work for v or t style functions,
> but we didn't try.
>
> VARIADIC functions are rare IMO and I had never heard of functions with
> TABLE parameters before.
>

yep, they're both rare.

>>
>> Also, if you have functions with multiple variables, but no named
>> parameters, you end up with things like
>> Notice: Undefined offset: 1 in
>> /Users/robert/devel/phppgadmin/phppgadmin/functions.php on line 267
>>
>> So I wrapped the switch in an isset call.
>
> That sounds good!
> BTW, we never use unnamed arguments anymore, so I simply didn't check that.
> The patch is intended to scratch a nasty itch for ourselves, not to fix any
> bugs that may exist in other situations.
>

Sure; I'm just trying to limit the cases where your patch might
introduce new bugs.

>
>> You can view the current version of the patch on my gtihub at
>>
>> https://github.com/xzilla/phppgadmin/commit/8ebf7562291f7018dedfa1ebb47290bb77b86087
>>
>> Feel free to check out from my repo if you want to test it. It'd be
>> nice to fix up the named parameters on OUT/returns stuff, but
>> otherwise I think it's pretty much ready (although if anyone has pre
>> 9.3 versions of postgres handy for testing, it'd be nice to verify
>> this works there as well)
>
>
> How do I check out from your repo?
>

There are some instructions in the side bar of
https://github.com/xzilla/phppgadmin, but basically do:

git clone gi...@gi...:xzilla/phppgadmin.git

and then you'll need to set up the conf/config.inc.php if you want to
test it. if you make changes, just run git diff to see them, and you
can create a file and send that, ie.

git diff > mychanges_1.txt

and then email mychanges_1.txt

> There is an error in the patch I suggested. In doProperties the variable is
> named funcdata instead of fndata. This is wrong on the new line number 256
> based on the webpage you refer above.
> Effect is that the properties show the wrong (number of) parameters. Simply
> changing fndata to funcdata solves this.
>

Ah! This clears up a bit of the problem I was having with INOUT vs.
OUT. So that is fixed; thanks! I think the only question remains is
thoughts on what to do with the returns field (right now you see both
OUT params in arguments, and you see a return field entry; is that
mis-leading?


Robert Treat
play: xzilla.net
work: omniti.com

Re: [ppa-dev] patch for supporting functions with output parameters

[Bert T. <bt...@br...>]

> At some point I think this might break on older versions, either
> without inout params and that field is missing (maybe?) or the lack of
> array_agg built in, which iirc is pre 8.4. So, I pushed the existing
> getFunction code back to Postgres83.php before making your
> modifications in Postgres.php

Sounds good to me!

> this is a little weird, because afaict OUT parameters never get
> display (they are just dumped to the returns box), although INOUT are.

I don't understand you. It is not dumped to the returns box, but in the 
"Arguments" box.

> This make me wonder what we would see with v or t style functions
> (didn't have any handy to test on, do you have any?) I think this
> could probably use more work to make RETURNS section display OUT with
> param name. (Not a show stopper, but you should look into it).

I cannot think of a reason why it wouldn't work for v or t style 
functions, but we didn't try.

VARIADIC functions are rare IMO and I had never heard of functions with 
TABLE parameters before.

>
> Also, if you have functions with multiple variables, but no named
> parameters, you end up with things like
> Notice: Undefined offset: 1 in
> /Users/robert/devel/phppgadmin/phppgadmin/functions.php on line 267
>
> So I wrapped the switch in an isset call.

That sounds good!
BTW, we never use unnamed arguments anymore, so I simply didn't check 
that. The patch is intended to scratch a nasty itch for ourselves, not 
to fix any bugs that may exist in other situations.

> You can view the current version of the patch on my gtihub at
> https://github.com/xzilla/phppgadmin/commit/8ebf7562291f7018dedfa1ebb47290bb77b86087
>
> Feel free to check out from my repo if you want to test it. It'd be
> nice to fix up the named parameters on OUT/returns stuff, but
> otherwise I think it's pretty much ready (although if anyone has pre
> 9.3 versions of postgres handy for testing, it'd be nice to verify
> this works there as well)

How do I check out from your repo?

There is an error in the patch I suggested. In doProperties the variable 
is named funcdata instead of fndata. This is wrong on the new line 
number 256 based on the webpage you refer above.
Effect is that the properties show the wrong (number of) parameters. 
Simply changing fndata to funcdata solves this.

Regards,
Bert

Re: [ppa-dev] patch for supporting functions with output parameters

[Robert T. <ro...@xz...>]

I reviewed the patch this morning, some thoughts in-line.


On Thu, Feb 20, 2014 at 9:49 AM, Bert Thomas <bt...@br...> wrote:
> Hi,
>
> My team and I use phpPgAdmin on a daily basis. One feature that lacked
> has always been very annoying. Today I decided to dive into it and
> patched it.
>
> The code that reads a function from the database only handles input
> parameters. This is because column proargtypes of pg_proc is used. This
> column only includes input and inout parameters. Even though all
> parameter names are available in the code, only the ones with a type are
> shown in the phpPgAdmin pages.
>
> pg_proc includes a column named 'proallargtypes' that includes the types
> for all parameters, but only in case outputparameters are used. When
> only inputparameters are available, this column contains a NULL value.
>
> First patch I made was a change to the getFunction() function in
> Postgres.php to include all types and the parameter 'modes' (in, out,
> inout, etc):
>

At some point I think this might break on older versions, either
without inout params and that field is missing (maybe?) or the lack of
array_agg built in, which iirc is pre 8.4. So, I pushed the existing
getFunction code back to Postgres83.php before making your
modifications in Postgres.php

> function getFunction($function_oid) {
>      $this->clean($function_oid);
>
>      $sql = "
>        SELECT
>          pc.oid AS prooid, proname, pg_catalog.pg_get_userbyid(proowner)
> AS proowner,
>          nspname as proschema, lanname as prolanguage, procost, prorows,
>          pg_catalog.format_type(prorettype, NULL) as proresult, prosrc,
>          probin, proretset, proisstrict, provolatile, prosecdef,
>          pg_catalog.oidvectortypes(pc.proargtypes) AS proarguments,
>          proargnames AS proargnames,
>          pg_catalog.obj_description(pc.oid, 'pg_proc') AS procomment,
>          proconfig,
>          (select array_agg( (select typname from pg_type pt where pt.oid
> = p.oid) ) from unnest(proallargtypes) p) proallarguments,
>          proargmodes
>        FROM
>          pg_catalog.pg_proc pc, pg_catalog.pg_language pl,
>          pg_catalog.pg_namespace pn
>        WHERE
>          pc.oid = '{$function_oid}'::oid AND pc.prolang = pl.oid
>          AND pc.pronamespace = pn.oid
>        ";
>
>      return $this->selectSet($sql);
>    }
>
> (replaced tab with 2 spaces for email readability)
>
> Then, in functions.php I updated the named parameter handling in doEdit
> and doProperties. The later uses a slightly other variable name, but the
> pattern is exactly the same:
>
> if ($data->hasNamedParams()) {
>    if( isset($fndata->fields['proallarguments'])  ) {
>      $args_arr = $data->phpArray($fndata->fields['proallarguments']);
>    } else {
>      $args_arr = explode(', ', $fndata->fields['proarguments']);
>    }
>
>    $names_arr = $data->phpArray($fndata->fields['proargnames']);
>    $modes_arr = $data->phpArray($fndata->fields['proargmodes']);
>    $args = '';
>    $i = 0;
>    for ($i = 0; $i < sizeof($args_arr); $i++) {
>      if ($i != 0) $args .= ', ';
>      switch($modes_arr[$i]) {
>      case 'i' : $args .= " IN "; break;
>      case 'o' : $args .= " OUT "; break;
>      case 'b' : $args .= " INOUT "; break;
>      case 'v' : $args .= " VARIADIC "; break;
>      case 't' : $args .= " TABLE "; break;
>      }

this is a little weird, because afaict OUT parameters never get
display (they are just dumped to the returns box), although INOUT are.
This make me wonder what we would see with v or t style functions
(didn't have any handy to test on, do you have any?) I think this
could probably use more work to make RETURNS section display OUT with
param name. (Not a show stopper, but you should look into it).

Also, if you have functions with multiple variables, but no named
parameters, you end up with things like
Notice: Undefined offset: 1 in
/Users/robert/devel/phppgadmin/phppgadmin/functions.php on line 267

So I wrapped the switch in an isset call.


>      if (isset($names_arr[$i]) && $names_arr[$i] != '') {
>        $data->fieldClean($names_arr[$i]);
>        $args .= '"' . $names_arr[$i] . '" ';
>      }
>      $args .= $args_arr[$i];
>    }
> }
>
> I don't understand git, it's too difficult for me. Therefore I post my
> patch this way.
>

:-)

You can view the current version of the patch on my gtihub at
https://github.com/xzilla/phppgadmin/commit/8ebf7562291f7018dedfa1ebb47290bb77b86087

Feel free to check out from my repo if you want to test it. It'd be
nice to fix up the named parameters on OUT/returns stuff, but
otherwise I think it's pretty much ready (although if anyone has pre
9.3 versions of postgres handy for testing, it'd be nice to verify
this works there as well)


Robert Treat
conjecture: xzilla.net
consulting: omniti.com

Re: [ppa-dev] patch for supporting functions with output parameters

[Robert T. <ro...@xz...>]

Hi Bert,

Thanks for the contribution. Do you mind if I ask what OS / Editor you use?

Robert Treat

On Thu, Feb 20, 2014 at 9:49 AM, Bert Thomas <bt...@br...> wrote:
> Hi,
>
> My team and I use phpPgAdmin on a daily basis. One feature that lacked
> has always been very annoying. Today I decided to dive into it and
> patched it.
>
> The code that reads a function from the database only handles input
> parameters. This is because column proargtypes of pg_proc is used. This
> column only includes input and inout parameters. Even though all
> parameter names are available in the code, only the ones with a type are
> shown in the phpPgAdmin pages.
>
> pg_proc includes a column named 'proallargtypes' that includes the types
> for all parameters, but only in case outputparameters are used. When
> only inputparameters are available, this column contains a NULL value.
>
> First patch I made was a change to the getFunction() function in
> Postgres.php to include all types and the parameter 'modes' (in, out,
> inout, etc):
>
> function getFunction($function_oid) {
>      $this->clean($function_oid);
>
>      $sql = "
>        SELECT
>          pc.oid AS prooid, proname, pg_catalog.pg_get_userbyid(proowner)
> AS proowner,
>          nspname as proschema, lanname as prolanguage, procost, prorows,
>          pg_catalog.format_type(prorettype, NULL) as proresult, prosrc,
>          probin, proretset, proisstrict, provolatile, prosecdef,
>          pg_catalog.oidvectortypes(pc.proargtypes) AS proarguments,
>          proargnames AS proargnames,
>          pg_catalog.obj_description(pc.oid, 'pg_proc') AS procomment,
>          proconfig,
>          (select array_agg( (select typname from pg_type pt where pt.oid
> = p.oid) ) from unnest(proallargtypes) p) proallarguments,
>          proargmodes
>        FROM
>          pg_catalog.pg_proc pc, pg_catalog.pg_language pl,
>          pg_catalog.pg_namespace pn
>        WHERE
>          pc.oid = '{$function_oid}'::oid AND pc.prolang = pl.oid
>          AND pc.pronamespace = pn.oid
>        ";
>
>      return $this->selectSet($sql);
>    }
>
> (replaced tab with 2 spaces for email readability)
>
> Then, in functions.php I updated the named parameter handling in doEdit
> and doProperties. The later uses a slightly other variable name, but the
> pattern is exactly the same:
>
> if ($data->hasNamedParams()) {
>    if( isset($fndata->fields['proallarguments'])  ) {
>      $args_arr = $data->phpArray($fndata->fields['proallarguments']);
>    } else {
>      $args_arr = explode(', ', $fndata->fields['proarguments']);
>    }
>
>    $names_arr = $data->phpArray($fndata->fields['proargnames']);
>    $modes_arr = $data->phpArray($fndata->fields['proargmodes']);
>    $args = '';
>    $i = 0;
>    for ($i = 0; $i < sizeof($args_arr); $i++) {
>      if ($i != 0) $args .= ', ';
>      switch($modes_arr[$i]) {
>      case 'i' : $args .= " IN "; break;
>      case 'o' : $args .= " OUT "; break;
>      case 'b' : $args .= " INOUT "; break;
>      case 'v' : $args .= " VARIADIC "; break;
>      case 't' : $args .= " TABLE "; break;
>      }
>      if (isset($names_arr[$i]) && $names_arr[$i] != '') {
>        $data->fieldClean($names_arr[$i]);
>        $args .= '"' . $names_arr[$i] . '" ';
>      }
>      $args .= $args_arr[$i];
>    }
> }
>
> I don't understand git, it's too difficult for me. Therefore I post my
> patch this way.
>
> Regards,
> Bert
>
> ------------------------------------------------------------------------------
> Managing the Performance of Cloud-Based Applications
> Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
> Read the Whitepaper.
> http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
> _______________________________________________
> phpPgAdmin-devel mailing list
> php...@li...
> https://lists.sourceforge.net/lists/listinfo/phppgadmin-devel

[ppa-dev] patch for supporting functions with output parameters

[Bert T. <bt...@br...>]

Hi,

My team and I use phpPgAdmin on a daily basis. One feature that lacked 
has always been very annoying. Today I decided to dive into it and 
patched it.

The code that reads a function from the database only handles input 
parameters. This is because column proargtypes of pg_proc is used. This 
column only includes input and inout parameters. Even though all 
parameter names are available in the code, only the ones with a type are 
shown in the phpPgAdmin pages.

pg_proc includes a column named 'proallargtypes' that includes the types 
for all parameters, but only in case outputparameters are used. When 
only inputparameters are available, this column contains a NULL value.

First patch I made was a change to the getFunction() function in 
Postgres.php to include all types and the parameter 'modes' (in, out, 
inout, etc):

function getFunction($function_oid) {
     $this->clean($function_oid);

     $sql = "
       SELECT
         pc.oid AS prooid, proname, pg_catalog.pg_get_userbyid(proowner) 
AS proowner,
         nspname as proschema, lanname as prolanguage, procost, prorows,
         pg_catalog.format_type(prorettype, NULL) as proresult, prosrc,
         probin, proretset, proisstrict, provolatile, prosecdef,
         pg_catalog.oidvectortypes(pc.proargtypes) AS proarguments,
         proargnames AS proargnames,
         pg_catalog.obj_description(pc.oid, 'pg_proc') AS procomment,
         proconfig,
         (select array_agg( (select typname from pg_type pt where pt.oid 
= p.oid) ) from unnest(proallargtypes) p) proallarguments,
         proargmodes
       FROM
         pg_catalog.pg_proc pc, pg_catalog.pg_language pl,
         pg_catalog.pg_namespace pn
       WHERE
         pc.oid = '{$function_oid}'::oid AND pc.prolang = pl.oid
         AND pc.pronamespace = pn.oid
       ";

     return $this->selectSet($sql);
   }

(replaced tab with 2 spaces for email readability)

Then, in functions.php I updated the named parameter handling in doEdit 
and doProperties. The later uses a slightly other variable name, but the 
pattern is exactly the same:

if ($data->hasNamedParams()) {
   if( isset($fndata->fields['proallarguments'])  ) {
     $args_arr = $data->phpArray($fndata->fields['proallarguments']);
   } else {
     $args_arr = explode(', ', $fndata->fields['proarguments']);
   }

   $names_arr = $data->phpArray($fndata->fields['proargnames']);
   $modes_arr = $data->phpArray($fndata->fields['proargmodes']);
   $args = '';
   $i = 0;
   for ($i = 0; $i < sizeof($args_arr); $i++) {
     if ($i != 0) $args .= ', ';
     switch($modes_arr[$i]) {
     case 'i' : $args .= " IN "; break;
     case 'o' : $args .= " OUT "; break;
     case 'b' : $args .= " INOUT "; break;
     case 'v' : $args .= " VARIADIC "; break;
     case 't' : $args .= " TABLE "; break;
     }
     if (isset($names_arr[$i]) && $names_arr[$i] != '') {
       $data->fieldClean($names_arr[$i]);
       $args .= '"' . $names_arr[$i] . '" ';
     }
     $args .= $args_arr[$i];
   }
}

I don't understand git, it's too difficult for me. Therefore I post my 
patch this way.

Regards,
Bert

Re: [ppa-dev] ppa51 bug report: pagination on select report

[phb07 <ph...@ap...>]

Szabolcs,

Thanks a lot for having shared this patch.
I hope the correction will be included soon in the core.

Philippe.

Le 10/02/2014 02:50, Szabolcs Hubai a écrit :
> Hi all,
>
>
> another STR is:
> 1. Select database
> 2. Select "SQL" tab
> 3. Make sure that "Paginate results" is checked
> 4. Execute: SELECT * FROM generate_series(1,100);
> 5. Click on any of the pagination links (Next, Last,...)
> 6a. Notice the "could not find the query!!" string on the top of page
> 6b. Notice that all the rows were fetched
>
>
> The problem is in Misc::printPages, it couldn't generate a link to
> point back to 'display.php'.
> Therefore where 'display.php' is included (tables.php and sql.php),
> the generated links are pointing
> to the original script instead to 'display.php'.
>
> This bug introduced by leaving (optimizing?) out the script name from
> printPage's $url/$gets parameter in commit e1a5b9c54f3e.
>
>
> Phillippe,
> if you are interested, then you could try the fix on my bugfix branch
> [2] or just apply the attached patch!
>
> ioguix,
> as you can see [3], I simply extended the Misc::printPages function
> with a "$script" parameter.
> Is this hotfix enough good for a Pull Request, or there are nicer and
> simpler fix?
> Comments and feedbacks are welcome!
>
>
> Cheers,
> Szabolcs
>
>
>
> [1]: https://github.com/phppgadmin/phppgadmin/commit/e1a5b9c54f3ea112a0ce5b2f0f3a0ace9583a354#diff-d7a651385f4ca725178b8cccd2bd50ddL533
> [2]: https://github.com/xabolcs/phppgadmin/tree/branch-bug-displayphp-shoud-pass
> [3]: https://github.com/xabolcs/phppgadmin/compare/branch-bug-displayphp-shoud-pass
>
> On 2013.06.20. 9:53, phb07 wrote:
>> Hi all,
>>
>> One of my customers reported to me a quite severe bug using ppa 5.1.0.
>> As it seems to be easily reproductible, here are the steps to see it.
>>
>> 1) Select a database
>> 2) Select a schema and choose the "Tables" tab -> the tables list appears
>> 3) For one table having a lot of rows, click on the "Select" button from
>> the actions list -> the columns list with its filtering conditions form
>> appears
>> 4) Select some or all columns to display and click on the "Select"
>> button at the bottom of the page -> the selected rows are displayed,
>> with pagination links if all rows cannot be displayed at once.
>> 5) Click on any of the pagination links (Next, Last,...) -> instead of
>> navigating on the results report, we directly go back to the tables list !
>>
>> Has this never been reported yet ?
>> I checked this morning that this bug exists on the current ppa HEAD version.
>>
>> Best regards.
>> Philippe Beaudoin.
>>
>>
>> ------------------------------------------------------------------------------
>> Managing the Performance of Cloud-Based Applications
>> Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
>> Read the Whitepaper.
>> http://pubads.g.doubleclick.net/gampad/clk?id=121051231&iu=/4140/ostg.clktrk
>>
>>
>> _______________________________________________
>> phpPgAdmin-devel mailing list
>> php...@li...
>> https://lists.sourceforge.net/lists/listinfo/phppgadmin-devel

Re: [ppa-dev] ppa51 bug report: pagination on select report

[Szabolcs H. <sz...@gm...>]

Hi all,


another STR is:
1. Select database
2. Select "SQL" tab
3. Make sure that "Paginate results" is checked
4. Execute: SELECT * FROM generate_series(1,100);
5. Click on any of the pagination links (Next, Last,...)
6a. Notice the "could not find the query!!" string on the top of page
6b. Notice that all the rows were fetched


The problem is in Misc::printPages, it couldn't generate a link to
point back to 'display.php'.
Therefore where 'display.php' is included (tables.php and sql.php),
the generated links are pointing
to the original script instead to 'display.php'.

This bug introduced by leaving (optimizing?) out the script name from
printPage's $url/$gets parameter in commit e1a5b9c54f3e.


Phillippe,
if you are interested, then you could try the fix on my bugfix branch
[2] or just apply the attached patch!

ioguix,
as you can see [3], I simply extended the Misc::printPages function
with a "$script" parameter.
Is this hotfix enough good for a Pull Request, or there are nicer and
simpler fix?
Comments and feedbacks are welcome!


Cheers,
Szabolcs



[1]: https://github.com/phppgadmin/phppgadmin/commit/e1a5b9c54f3ea112a0ce5b2f0f3a0ace9583a354#diff-d7a651385f4ca725178b8cccd2bd50ddL533
[2]: https://github.com/xabolcs/phppgadmin/tree/branch-bug-displayphp-shoud-pass
[3]: https://github.com/xabolcs/phppgadmin/compare/branch-bug-displayphp-shoud-pass

On 2013.06.20. 9:53, phb07 wrote:
> Hi all,
>
> One of my customers reported to me a quite severe bug using ppa 5.1.0.
> As it seems to be easily reproductible, here are the steps to see it.
>
> 1) Select a database
> 2) Select a schema and choose the "Tables" tab -> the tables list appears
> 3) For one table having a lot of rows, click on the "Select" button from
> the actions list -> the columns list with its filtering conditions form
> appears
> 4) Select some or all columns to display and click on the "Select"
> button at the bottom of the page -> the selected rows are displayed,
> with pagination links if all rows cannot be displayed at once.
> 5) Click on any of the pagination links (Next, Last,...) -> instead of
> navigating on the results report, we directly go back to the tables list !
>
> Has this never been reported yet ?
> I checked this morning that this bug exists on the current ppa HEAD version.
>
> Best regards.
> Philippe Beaudoin.

[ppa-dev] Allow users to see errors raised on transaction commit

[Karl O. P. <ko...@me...>]

Hi,

This branch:

https://github.com/kpinc/phppgadmin/commits/commit_triggers

allows the user to see errors raised on transaction
commit.

It is not well tested.

Regards,

Karl <ko...@me...>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

[ppa-dev] Theme based on server, database or user

[Jehan-Guillaume de R. <jg...@da...>]

Hello,

Here is a patch that allows to use different theme depending on servers,
users and databases

The priority order is :
  * the theme defined for a server
  * the theme defined for a database apply over the server one
  * the theme defined for a user apply over the database one

This patch answers a feature request we had from a customer, willing to
have different theme between production clusters and dev ones.

On github:
  https://github.com/ioguix/phppgadmin/tree/dyn_themes

Any feedback before I push?
-- 
Jehan-Guillaume de Rorthais
http://www.dalibo.com

Re: [ppa-dev] Pull request

[Karl O. P. <ko...@me...>]

On 07/12/2013 09:36:27 AM, Tomasz Pala wrote:

> but please, just review them and apply at least the obvious parts:

I confess to feeling neglected too.  As someone who's
sporadically intensely involved in ppa I'm not
in a position to step up and assume continuous
responsibility as a(/the?) ppa maintainer so
I can hardly complain when there's not continuous
coverage.

This is not to say that I don't wish for
a better situation.

Could we, conceivably, improve patch throughput
by having a mutual review/signoff process
in a fashion similar to that of pg?
This probably won't entirely solve the problem;
I produce piles of code in short periods
and then become somewhat unavailable
and I can't see there being enough
interest in reviewing to go all
my patches.  But it might be helpful
anyway.

If nothing else this might give people
concrete feedback so they know how
to move forward with their patches.

Regards,

Karl <ko...@me...>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

[ppa-dev] Pull request

[Tomasz P. <go...@po...>]

Hello,

a few days ago I've send pull request for my changes regarding:

1. JS calendar code (ignored for 4 years),
2. Blue/Green theme updates, mostly missing parts (ignored for 2 years),
3. XTree behaviour fixes,
4. input data-loss fix,
5. interface decluttering and keyboard shortcuts.

This mail is just a remainder, that there is something like pull in git
and now it's much easier to incorporate contributed patches into
mainstream. I am aware, that:

1. my JS calendar is 'bad' (dunno however why) and you'd prefer having
jQuery one (which wouldn't be better in any way, and last 4 years prove
that it will probably never exist),

2. my Blue/Green theme isn't used by any of you, so you don't need to
update it's missing parts,

3. XTree code is about to be abandoned and rewritten, so there's no need
to improve it (yeah, jQuery again, maybe this decade),

4. people get used to retyping their data, after all it's not gone but
available within error message.

but please, just review them and apply at least the obvious parts:

https://github.com/phppgadmin/phppgadmin/pull/19

-- 
Tomasz Pala <go...@pl...>

[ppa-dev] ppa51 bug report: pagination on select report

[phb07 <ph...@ap...>]

Hi all,

One of my customers reported to me a quite severe bug using ppa 5.1.0.
As it seems to be easily reproductible, here are the steps to see it.

1) Select a database
2) Select a schema and choose the "Tables" tab -> the tables list appears
3) For one table having a lot of rows, click on the "Select" button from 
the actions list -> the columns list with its filtering conditions form 
appears
4) Select some or all columns to display and click on the "Select" 
button at the bottom of the page -> the selected rows are displayed, 
with pagination links if all rows cannot be displayed at once.
5) Click on any of the pagination links (Next, Last,...) -> instead of 
navigating on the results report, we directly go back to the tables list !

Has this never been reported yet ?
I checked this morning that this bug exists on the current ppa HEAD version.

Best regards.
Philippe Beaudoin.

[ppa-dev] Fix re-login in popup window

[Karl O. P. <ko...@me...>]

Hi,

I've a new branch on github: popuplogin

This branches off 5.1.

https://github.com/kpinc/phppgadmin/commits/popuplogin

It fixes what happens when you click on one of the
top right popup links when logged out.

Test by opening two tabs.  Login to the same
server in ppa in both tabs.  Press the logout
button in one tab, then in the other click
on one of the top right popup links.  The
popup will display a login.  This patch
fixes what displays in the popup after login.

Regards,

Karl <ko...@me...>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

P.S.  I think I'm going to have to rebase my
import93 branch to use this patch, since it
gets php errors under the conditions this branch
fixes.  I expect to have to resolve one conflict.
I'd appreciate knowing if this is going
to go into head or otherwise how I should rebase.

Thanks.

Re: [ppa-dev] The future of the xtree code in ppa

[Karl O. P. <ko...@me...>]

On 06/13/2013 05:32:25 PM, Jehan-Guillaume (ioguix) de Rorthais wrote:
> On 03/06/2013 04:27, Karl O. Pinc wrote:
> > Hi,
> > 
> > I've been looking at the xtree "database browser" code
> > and have begun some extensions of it to support the
> > ability to see views mixed in with tables, or not.
> 
> Mh. Why would you want to have table and views mixed all together?

Mh.  Why would you want them separated?  ;-)  My users can't tell
the difference, and shouldn't have to.

You can, by the by, add foreign data wrappers into the mix as
well.  They also look like tables.

> 
> I prefer having small branches in the tree, with table, views,
> matviews
> in their own branches, than having all of them in a long, big, slow 
> to
> load branch, not to mention scrolling much more to find your object 
> in
> the branch...Moreover, the current tree is pretty close to the
> pgAdmin's
> one, which helps user to switch between tools without too much
> differences.

No worries.  I'm not proposing getting rid of having separate
branches for tables, views, matviews, and fdws.  What I'm
proposing is being able to let the user choose whether to see
separate branches or not.

If you care about the underlying storage model you can see
them separately.  If you don't and all you want to do is
see what data exists so that you can query/browse/insert/whatever
then you can see them all mixed together.  As a db
admin I want to be able to substitute a table for a view
for a fdw and vice versa without confusing the less 
sophisticated user who just wants to be able to get to
his data.  Again, I'm not removing anything.  I'm adding.
Do you guys have a problem with this as a feature?

FWIW, to avoid big, slow, long branches I have the build
system create schemas which contain views into various
areas of the larger db, by topic or whatever.
These scheamas contain nothing but views to deliver
just those portions of the db to the targeted users.
This has worked increasingly well as views have
become updatable and otherwise more like tables.
(Although this is not the motivation for the above.)
(I thought about frobbing ppa to do this, but views
seemed the best approach.)  Basically, I've gotten
to the point where there's enough plain old tables 
in the db that people get dazed.

> 
> > The last message from ioguix disturbed me and got me
> > thinking.  I don't want to work against the project's
> > direction and I've vague memories of discussions
> > regarding javascript and the db browser tree's future.
> > 
> > I want to discuss whether there is any thought that the xtree
> > code might be replaced before I put in more work.  I've looked
> > back through the mailing list and see no such discussion,
> > but for some reason I think this may have been
> > discussed on IRC,
> 
> Yeah, it has been discussed here and there...
> 
> > perhaps with a thought to using
> > a jQuery plugin like jstree -- in the context of
> > needing a newer jQuery.  (jstree requires
> > jQuery 1.9, I think.)  Since I've patches to remove
> > some dependencies on jQuery 1.8 this might be in
> > the offing.
> 
> Yes, that is a project floating around, replacing the current tree
> with
> a jquery one.

Any thoughts on which one?

> 
> > I lean toward the "if it ain't broke don't fix it" side
> > of things, especially since it would be non-trivial
> > and I think we'd require extensions beyond a basic
> > "tree browser" to support the functionality we
> > currently have.
> 
> The problem about xtree is that it is a dead wood stick. It is not
> maintained anymore, we already fixed a bunch of bug in there and
> there's
> still some more (like the auto-expand opening differents branches on
> refresh with new objects).
> 
> Moreover, since we moved to jquery, it would be much better now to 
> use
> a
> plugin which is a maintained project and give much more flexibilty in
> code. Xtree code is just horrible to deal with.
> 
> > Please give me some guidance here.  I don't want
> > to put work into xtree that won't get committed
> > because xtree isn't wanted.  (You may not like
> > the code for other reasons, but that's another
> > story.)
> 
> Well, I'm not the only one speaking for the project, I would like to
> know Robert's point of view. But I think xtree must die.

I'm willing to kill xtree, as part of my larger plan to (optionally)
show the user relations and ignore the underlying storage (per above).
But I'd need to know what to replace it with.  jstree works for me.
Do people want something else?

More to the point, could I get a patch that allows the user to
choose between seeing either all relations together or split
into tables, views, matviews, fdws into PPA?  Or is this out
of the question?
 
> On the a side note, I would really like to see the frames die as well
> and this tree integrated in the main page with a switch to show/hide
> it.
> Not to mention the huge ergonomic win, we have a patch to set a
> different PPA theme based on the serveur or db or user you are on. It
> should really be just applying a css stylesheet, but we have to mess
> with frames and a bunch of js to refresh to tree as well...beurk.

+1  But one thing at a time.

It'd also be nice to have separate state (cookies) for different
windows/tabs in the browser.  This might obviate the need for
per server/db themes -- depending on cookie lifetime, etc.
Anyway it's something to think through when the time comes.

(Backend theme state is powerful, but also something of intrusion
on the backend.  I too thought about backend state when
it comes to solving the problem of "this db has a confusing
number of tables".  If we ever start keeping state in the
backend I may want to propose some sort of table categorization
solution.)


Karl <ko...@me...>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

Re: [ppa-dev] Fixes to js/database.js

[Karl O. P. <ko...@me...>]

On 06/13/2013 05:13:53 PM, Jehan-Guillaume (ioguix) de Rorthais wrote:
> Just to sum up:
> 
> On 14/06/2013 00:09, Jehan-Guillaume (ioguix) de Rorthais wrote:
> > On 30/05/2013 04:28, Karl O. Pinc wrote:
> >> Just added a 5th patch to the branch to set the
> >> correct cursor graphic.
> 
> -1
> 
> >>> 1) Remove jQuery 1.8 .toggle() dependency from js/database.js
> 
> Please, explain a bit more. I don't see toggle as deprecated in 
> jquery
> 1.8 or more, neither I understand the problem.

See toggle as depreciated in 1.8, removed in 1.9, here:

http://api.jquery.com/category/deprecated/deprecated-1.8/

Or for more detail:

http://api.jquery.com/toggle-event/

There is _another_ toggle, http://api.jquery.com/toggle/,
that toggles the css display element to display/hide,
but that's not the toggle that database.js is using.

> 
> and -1 for the <noscript> tag.
> 
> >>> 2) Add style to the links made active by js.
> 
> -1, the <a> tag should not have been removed in 1st patch.

Your points above all have to do with the <a> element.

Patch1 has got several things going on,
including the removal of the <a> element.
I might have made multiple patches, but I'm
pretty sure that would cause horrible brokenness
and be confusing itself.  Anyhow, I didn't try.

If you _don't_ remove the <a> element as part
of patch1 (at least in
my browser and while I'm not steeped in the DOM
event model I don't see why it shouldn't happen
all the time) is that clicking on "stop" runs the
js which stops the redisplay and then the event
percolates down to the <a> tag and the page
reloads and starts the repeated refresh all over
again.  So, there's no stopping the refresh.

This was quite spooky and was the part that
was very annoying to figure out.

One of the other things that patch1 does, besides
replacing toggle() with one(), is to use setInterval()
instead of repeatedly calling setTimeout()
on each refresh to setup the next refresh.
(The odd thing is that the current code calls
clearInterval() to clear the setTimeout() call
instead of calling clearTimeout().)  Whether
this has anything to do with why the current
code actually stops -- never actually
delivering the click event to the <a> element--
or whether it's the jQuery toggle() that's
frobbing the event bubbling
I don't know.  Or maybe it's some sort
of race condition like 4) below.  

Regardless, it's more clear to use one() to toggle between
two states and let setInterval() manage
the "animation".   

(For that matter one()
seems good for toggling between N states.
At least it seems that way to me.  I don't know
enough jQuery/js to be certain this is absolutely
he most elegant way to do it.  It's a way
of writing a state machine as code and there
might be an abstract state machine approach
that's better.

Gotta wonder why toggle() is depreciated/removed.)

I'm not all that interested right now in why
the current code isn't broken.

The fundamental uglyness is that ppa is using <a> for
styling in the js case and for fallback to the no-js case.
Something mysterious is presently happening
to keep the <a> from reloading the page in the js case.
Better to use <noscript> for the no-js case
and use css for styling.

> 
> >>> 3) Have refresh happen instantly when "start"
> >>> is clicked.
> 
> +1 for this one
> 
> >>> 4) Fix bug where clicking on "stop" would
> >>> display an error message.
> 
> I don't understand this one.

If the stop link is clicked _while_ the ajax request
is in transit then the request fails
and you get an error message.  But the user caused
the request to fail/abort so there should be
no error message.

Throttle your connection to the http server and
try it.  (Or just keep poking it with a stick
and get lucky.)



Karl <ko...@me...>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

Re: [ppa-dev] Fix bugs in <= 9.1 processes page

[Jehan-Guillaume (i. de R. <io...@fr...>]

Good catch! Thank you for the patch!

(and mea culpa :))

On 24/05/2013 18:50, Karl O. Pinc wrote:
> Hi,
> 
> I've a new branch "sigtidy" that fixes bugs in the
> <= 9.1 process page.  The database string is cleaned
> and processes are restricted to the given db (when
> one is specified in the call.)
> 
> https://github.com/kpinc/phppgadmin/commits/sigtidy
> 
> I looked back at the history and can't tell why the
> bugs crept in.
> 
> I've tested with 8.4 and verified (on IRC with RhodiumToad)
> that using datname for the db name column is correct back to 7.4.
> 
> 1 patch:
> ------
> Processes: Fix <= 9.1 to show only db related processes & clean dbname.
>     
>     Fix bugs introduced in commit d571ecae7b by ioguix on 2012-09-21.
> 

Re: [ppa-dev] The future of the xtree code in ppa

[Jehan-Guillaume (i. de R. <io...@fr...>]

On 03/06/2013 04:27, Karl O. Pinc wrote:
> Hi,
> 
> I've been looking at the xtree "database browser" code
> and have begun some extensions of it to support the
> ability to see views mixed in with tables, or not.

Mh. Why would you want to have table and views mixed all together?

I prefer having small branches in the tree, with table, views, matviews
in their own branches, than having all of them in a long, big, slow to
load branch, not to mention scrolling much more to find your object in
the branch...Moreover, the current tree is pretty close to the pgAdmin's
one, which helps user to switch between tools without too much differences.

> The last message from ioguix disturbed me and got me
> thinking.  I don't want to work against the project's
> direction and I've vague memories of discussions
> regarding javascript and the db browser tree's future.
> 
> I want to discuss whether there is any thought that the xtree
> code might be replaced before I put in more work.  I've looked
> back through the mailing list and see no such discussion,
> but for some reason I think this may have been
> discussed on IRC,

Yeah, it has been discussed here and there...

> perhaps with a thought to using
> a jQuery plugin like jstree -- in the context of
> needing a newer jQuery.  (jstree requires
> jQuery 1.9, I think.)  Since I've patches to remove
> some dependencies on jQuery 1.8 this might be in
> the offing.

Yes, that is a project floating around, replacing the current tree with
a jquery one.

> I lean toward the "if it ain't broke don't fix it" side
> of things, especially since it would be non-trivial
> and I think we'd require extensions beyond a basic
> "tree browser" to support the functionality we
> currently have.

The problem about xtree is that it is a dead wood stick. It is not
maintained anymore, we already fixed a bunch of bug in there and there's
still some more (like the auto-expand opening differents branches on
refresh with new objects).

Moreover, since we moved to jquery, it would be much better now to use a
plugin which is a maintained project and give much more flexibilty in
code. Xtree code is just horrible to deal with.

> Please give me some guidance here.  I don't want
> to put work into xtree that won't get committed
> because xtree isn't wanted.  (You may not like
> the code for other reasons, but that's another
> story.)

Well, I'm not the only one speaking for the project, I would like to
know Robert's point of view. But I think xtree must die.

On the a side note, I would really like to see the frames die as well
and this tree integrated in the main page with a switch to show/hide it.
Not to mention the huge ergonomic win, we have a patch to set a
different PPA theme based on the serveur or db or user you are on. It
should really be just applying a css stylesheet, but we have to mess
with frames and a bunch of js to refresh to tree as well...beurk.

> 
> Regards,
> 

Re: [ppa-dev] Fixes to js/database.js

[Jehan-Guillaume (i. de R. <io...@fr...>]

Just to sum up:

On 14/06/2013 00:09, Jehan-Guillaume (ioguix) de Rorthais wrote:
> On 30/05/2013 04:28, Karl O. Pinc wrote:
>> Just added a 5th patch to the branch to set the
>> correct cursor graphic.

-1

>>> 1) Remove jQuery 1.8 .toggle() dependency from js/database.js

Please, explain a bit more. I don't see toggle as deprecated in jquery
1.8 or more, neither I understand the problem.

and -1 for the <noscript> tag.

>>> 2) Add style to the links made active by js.

-1, the <a> tag should not have been removed in 1st patch.

>>> 3) Have refresh happen instantly when "start"
>>> is clicked.

+1 for this one

>>> 4) Fix bug where clicking on "stop" would
>>> display an error message.

I don't understand this one.

Cheers,

/ioguix

Re: [ppa-dev] Fixes to js/database.js

[Jehan-Guillaume (i. de R. <io...@fr...>]

On 30/05/2013 04:28, Karl O. Pinc wrote:
> Hi,
> 
> Just added a 5th patch to the branch to set the
> correct cursor graphic.  (Should really probably
> be patch 3, but I'm not rebaseing and rewriting
> history.)

I guess you need to do such contortions because you droped the <a> tag
from your first patch e92014774f7 (removing jquery 1.8 dependencies).

I understand now why you tried to put a style on the <div> tag to make
it look like a link :)

> When examining this patch and caa5cff9 (patch 2)
> it's worth thinking about whether js created
> hot-spots should have their own css selector,
> as with foreign keys.  I've not thought about it
> much and it's not obvious to me that it's useful,
> nor obvious that it's useless, so I opted for
> simplicity.
> 
> See also:
> https://developer.mozilla.org/en-US/docs/Web/CSS/
> cursor#Browser_compatibility
> 
> On 05/29/2013 04:41:57 PM, Karl O. Pinc wrote:
>> Hi,
>>
>> I've a new branch "jquery18":
>>
>> https://github.com/kpinc/phppgadmin/commits/jquery18
>>
>> Uh, I branched this off HEAD, not 5.1.  There are
>> fixes, there are enhancements....
>>
>> I did not test with multiple browsers.  It's all jQuery
>> so it's supposed to work but it wouldn't hurt to
>> get more testing before committing.  Tested with FF 3.5.
>>
>>
>> There are 4 patches:
>>
>> 1) Remove jQuery 1.8 .toggle() dependency from js/database.js
>>
>> This turned out to be really annoying.  See the commit
>> message.  
>>
>> I strongly suspect that this fixes a bug
>> that causes js to constantly run, forever, even
>> when the refresh seems to be stopped.  Often
>> this seems to manifest itself in my browser as
>> an alert in the morning that says a script is not
>> responding.  On the other hand, maybe some other
>> random web page is doing that to me.
>>
>> This also re-orders when display is done and
>> when the ajax requests are started with an
>> eye toward making things less racy.  It probably
>> does not matter, but if you're interested
>> a 2nd set of eyes on this wouldn't hurt.
>>
>>
>> 2) Add style to the links made active by js.
>>
>>
>> 3) Have refresh happen instantly when "start"
>> is clicked.  
>>
>> The window.setInterval() docs
>> seem to deliberately leave unspecified
>> whether the function is called instantly
>> or the first call happens after a delay.
>> Regardless, this does no harm.  Right?
>> It makes my browser more snappy.
>>
>>
>> 4) Fix bug where clicking on "stop" would
>> display an error message.
>>
>> This is not a particularly elegant solution,
>> but it works.
>>
>> Regards,
>>

Re: [ppa-dev] Fixes to js/database.js

[Jehan-Guillaume (i. de R. <io...@fr...>]

On 29/05/2013 23:41, Karl O. Pinc wrote:
> 1) Remove jQuery 1.8 .toggle() dependency from js/database.js
> 
> This turned out to be really annoying.  See the commit
> message.

Do you have any pointers or more explanations about that ?

> I strongly suspect that this fixes a bug
> that causes js to constantly run, forever, even
> when the refresh seems to be stopped.  Often
> this seems to manifest itself in my browser as
> an alert in the morning that says a script is not
> responding.  On the other hand, maybe some other
> random web page is doing that to me.

I guess it might be random web pages. When I stop the auto-refresh,
using the profiler of firebug doesn't show me anything running.

Another comment about this commit. Why did you add <noscript> tags and
codes ? The current code already take care of this:
  * with no js activated, you just get a simple "refresh" link.
  * with js, the javascript code kicks off after the page and transform
this exact "refresh" link as a switch to start/stop refresh.

> 2) Add style to the links made active by js.

huh, why ? most of our links are not underscored. Why should this one
look different from others ?

> 3) Have refresh happen instantly when "start"
> is clicked.  
> 
> The window.setInterval() docs
> seem to deliberately leave unspecified
> whether the function is called instantly
> or the first call happens after a delay.
> Regardless, this does no harm.  Right?
> It makes my browser more snappy.

Ok for this one, but I don't think I can cherry-pick it right now as it
drift in the miffle of your first patch on this branch.

> 4) Fix bug where clicking on "stop" would
> display an error message.
> 
> This is not a particularly elegant solution,
> but it works.

Sorry, I'm not sure to understand this one, do you have a use case ?

Thank you,

/ioguix