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

[proposal] Aliased columns #21

Closed
wants to merge 2 commits into from

Conversation

mbrevda
Copy link
Contributor

@mbrevda mbrevda commented Jun 26, 2014

Problem

I'm writing a system that will allow a restful api consumer to lightly modify/restrict a query in a way that dose not require the author of a model to account for with any special behavior. The $crub object would be passed to a class that would auto-manipulate the query using passed url query parameters.

The proposed url query format would be like (w/ whitespace for readability):

api.example.com?
    $limit=4|5             // limit | offset
    &$sorts[]=col2         // column
    &$fields[]=col1        // column, will only return listed fields
    &$fields[]=col2        // column
    &$filter[]=col1|>|6    // column | operator | comparator
    &$filter[]=col2|=|hot  // column | operator | comparator

While some params are simple to implement (i.e. adding a limit/offset is as simple as parsing their value and calling $select->limit($limit)) there are others that are proving quite difficult. Ignoring property visibility in sqlqury's CRUD classes, here are some of situations encountered.

Given the following sql query:

SELECT 
    foo,
    bar,
    fooblue,
    yellow.baz as zzz
FROM 
    orange 
LEFT JOIN yellow AS y ON orange.foo = y.foo;
  1. Filtering on foo will cause mysql to complain of ambiguity
  2. Adding a table to the filter field ($filter[]=yellow.col2|=|hot), while removing ambiguity, violates the law of Leaky Abstraction. Additionally, it causes some headaches when trying to escape the user-supplied values by using bindValue()
  3. Auto appending the table name to the column before calling where() limits filters to the main table, restricting columns from a join from being filtered
  4. Removing columns to accommodate $fields requests is extremely difficult to do accurately without a complex convention (and even then).
  5. The consumer must request a filter on the DB field name, and cannot use the field name returned to him by the api. This forces him to be fully aware of the DB schema, removing separation of concerns.

Solution

The proposed solution would be to allow columns to be aliased for further reference. By way of example, here is what the api would look like:

$select
    ->cols([
        'foo',               // legacy/backwards compatible
        'bar as bbb',        // legacy/backwards compatible, will be auto parsed
        ['yellow.foo' => 'foo'], // new syntax, in the form of field => alias. Will be built as 'yellow.foo AS foo`,
        ['yellow.baz' => 'zzz']
    ])

This syntax will allow for any aliased field to be simply omitted from the query (a new removeCol() method will be necessary), and will allow for greater intelligence when trying to filter queries:

  1. Filtering would be requested on the alias (foo) which can then be resolved to yellow.foo by looking up the real field name in the $cols array.
  2. No need to reference the proper table
  3. No need to append the table name or try to automagically guess it
  4. Removing aliased fields would be a cinch ($select->removeCol($alias))
  5. By their very definition of being aliased, the modal will return fields by their aliased names. Should these be returned directly to the consumer, the consumer can use the same field for a $fields limitation.

I'm also open to other suggestions on how to go about this issue.

@mbrevda mbrevda changed the title Aliased columns [proposal] Aliased columns Jun 17, 2014
@pmjones
Copy link
Member

pmjones commented Jun 30, 2014

@mbrevda We have had colname => alias notation for a little while now but it has been undocumented. I have just added a very brief note about it as an option in the README under https://github.com/auraphp/Aura.SqlQuery#select. Would that do the trick for this particular PR?

@mbrevda
Copy link
Contributor Author

mbrevda commented Jun 30, 2014

No. The idea here is to store the aliases so that the columns list can be
later manipulated (which will be included in part two, as you requested).
In the current form, the columns and aliases are "compiled" in a way that
makes it extremely difficult to manipulate the list.
On Jun 30, 2014 8:56 PM, "Paul M. Jones" notifications@github.com wrote:

@mbrevda https://github.com/mbrevda We have had colname => alias
notation for a little while now but it has been undocumented. I have just
added a very brief note about it as an option in the README under
https://github.com/auraphp/Aura.SqlQuery#select. Would that do the trick
for this particular PR?


Reply to this email directly or view it on GitHub
#21 (comment).

@pmjones
Copy link
Member

pmjones commented Jun 30, 2014

Gotcha. So we really have two problems: (1) the => notation to keep them separate, but really (2) the fact that the original and alias are not stored as independent values. And the solution is to not-build the column string from those values until query-building time. Is that about it?

@mbrevda
Copy link
Contributor Author

mbrevda commented Jun 30, 2014

That's what's been submitted so far
On Jun 30, 2014 9:18 PM, "Paul M. Jones" notifications@github.com wrote:

Gotcha. So we really have two problems: (1) the => notation to keep them
separate, but really (2) the fact that the original and alias are not
stored as independent values. And the solution is to not-build the column
string from those values until query-building time. Is that about it?


Reply to this email directly or view it on GitHub
#21 (comment).

@pmjones
Copy link
Member

pmjones commented Jul 26, 2014

@mbrevda please take a look at this alternative implementation: #33

@pmjones
Copy link
Member

pmjones commented Aug 4, 2014

replaced by #33

@pmjones pmjones closed this Aug 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants