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 select with Alaises #6

Closed
shravass opened this issue Jul 4, 2017 · 6 comments
Closed

SQL select with Alaises #6

shravass opened this issue Jul 4, 2017 · 6 comments

Comments

@shravass
Copy link
Contributor

shravass commented Jul 4, 2017

In original documentation , Select query with aliases is given the following explanation:
https://github.com/allejo/PhpSoda/wiki/Working-with-SoQL-Queries

I came across two issues while working with aliases:

  1. When a select is given with alias, the $select statement generated has white spaces instead of %20. As a result, the HTTP request fails with response of 400 error code

eg: $selectWithAliases = array(
"origin" => "origin_type"
);

URL formed: https://.../.../...?$select=origin AS origin
Expected URL: https://.../.../...?$select=origin%20AS%20origin

  1. While trying the case "Not all column names must have an alias", the column given without alias are generated like origin AS , file_type AS, year as publication_year instead of origin, file_type, year as publication_year

eg: $selectWithAliases = array(
"origin",
"file_type",
"year" => "publication_year"
);

URL formed: https://.../.../...?$select=origin AS ,file_type AS ,year AS publication_year
Expected URL: https://.../.../...?$select=origin,filer_name,year AS publication_year

A patch is required in line 169 of src/SoqlQuery.php to fix these 2 issues:

$formattedValues[] = (is_string($key) && (!is_null($value))) ? rawurlencode(sprintf($format, trim($key), trim($value))) : (is_string($key) ? $key : $value);

(is_string($key) && (!is_null($value))) ? rawurlencode(sprintf($format, trim($key), trim($value))) : (is_string($key) ? $key : $value) will handle the following cases:

  1. Select with key and value ( column with alias )
  2. Select with key and null value( column without alias while other column may come with alias)
  3. Select with value ( column without alias in any column of select statement)
@allejo
Copy link
Owner

allejo commented Jul 4, 2017

The spaces in the "AS" clauses is confirmed

While trying the case "Not all column names must have an alias", the column given without alias are generated like origin AS , file_type AS, year as publication_year instead of origin, file_type, year as publication_year

I can't reproduce this behavior, do you have another snippet that shows it? Currently the ternary is checking if the key is a string where "origin" and "file_type" would give me integer keys.

$selectWithAliases = array(
  "origin",
  "file_type",
  "year" => "publication_year"
);

This is what I'm testing on PHP 7.1.6, is this an inconsistency in PHP versions?

$soql = new SoqlQuery();
$soql->select(array(
    'origin',
    'file_type',
    'year' => 'publication_year'
));

// $select=origin,file_type,year AS publication_year
$result = (string)$soql;

@shravass
Copy link
Contributor Author

shravass commented Jul 4, 2017

Hi,
Thank you for your immediate response.

With respect to point 2:
We are actually using a YAML SQL file and we parse the YAML file using symfony https://symfony.com/doc/current/components/yaml/yaml_format.html

We were not able to generate an array with mix of (key=>value and value) with the parser. We could only generate an array with (key=>value and key=> null) .

It would be great if you can add this additional check to handle the same. Also, let us know if you have any alternative approach to this problem.

@allejo
Copy link
Owner

allejo commented Jul 4, 2017

What's the generated structure look like from Symfony? I'm cool with adding the additional check 👍 Probably just need to convert it into an if statement since the nested ternaries are getting hard to understand

@shravass
Copy link
Contributor Author

shravass commented Jul 4, 2017

It generates the following array formats:

  1. array( "value", "value", "value");
  2. array("key" => "value" , "key" => "value");
  3. array("key" => null, "key" => "value");

We tried the combination of array("key" => "value" , "value") but Symfony is not able to recognize this format.

@allejo
Copy link
Owner

allejo commented Jul 4, 2017

Ah, that makes sense. In your PR, could you do the following and I'll merge it in:

  1. Add unit tests to test against both array structures:

    array(
      "origin",
      "file_type",
      "year" => "publication_year"
    );

    and

    array(
      "origin" => null,
      "file_type" => null,
      "year" => "publication_year"
    );
  2. Split up the nested ternaries into an if statement. Something like this?

    if (is_string($key) && !is_null($value))
    {
        $formattedValues[] = rawurlencode(sprintf($format, trim($key), trim($value)));
    }
    else
    {
        $formattedValues[] = is_string($key) ? $key : $value;
    }

@shravass
Copy link
Contributor Author

shravass commented Jul 4, 2017

I have completed the above. Hope the test cases are fine.
Thanks

@allejo allejo closed this as completed in #7 Jul 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants