catfan / Medoo

The lightweight PHP database framework to accelerate the development.
https://medoo.in
MIT License
4.84k stars 1.15k forks source link

Joining tables, selection of all fields not working #631

Closed tds4u closed 7 years ago

tds4u commented 7 years ago
$db_select = CLASS_DB::getInstance()->select(
    $this->tables['fields'].'(f)',
    array(
        '[>]'.$this->tables['profiles'].'(p)' => array('field_id')
    ),
    array(
        'f.*',
        'p.'.$this->idfields['profiles'].'(in_profile)'                     
    ),
    array(
        'AND' => array(
            'f.deleted' => 0,
            'f.active' => 1, 
            'f.isuserdata' => 0,
            'f.template_id' => $node_info['template_id']
        )
    )
);

Expected: SELECT "f".*,"p"."user_id" AS "in_profile" FROM "portal_userlist_fields" AS "f" LEFT JOIN "portal_userlist_profile" AS "p" USING ("field_id") WHERE "f"."deleted" = 0 AND "f"."active" = 1 AND "f"."isuserdata" = 0 AND "f"."template_id" = '1'

Generated: SELECT "f"."","p"."user_id" AS "in_profile" FROM "portal_userlist_fields" AS "f" LEFT JOIN "portal_userlist_profile" AS "p" USING ("field_id") WHERE "f"."deleted" = 0 AND "f"."active" = 1 AND "f"."isuserdata" = 0 AND "f"."template_id" = '1'

**Note that "f"."" is not "f".***

Andrews54757 commented 7 years ago

@tds4u Wildcards in column names are not supported. Its a bad practice to do so.

tds4u commented 7 years ago

@Andrews54757 If you have user field tables with dynamic fields for data than it's equal whether to join with * or list all table fields. Getting user preferences in this case needs all fields. So I can't select only needed fields. And first getting field names will result in additional query which also can be slow on MySQl.

Andrews54757 commented 7 years ago

@tds4u Okay then, I guess a little change could be made:

Change https://github.com/catfan/Medoo/blob/6798b9698ee60ee98a4d342485904322922b2b3b/src/Medoo.php#L356-L366

To

    protected function columnQuote($string)
    {
        preg_match('/(^#)?([a-zA-Z0-9_]*)\.([a-zA-Z0-9_\*]*)(\s*\[JSON\]$)?/', $string, $column_match);
        if (isset($column_match[ 2 ], $column_match[ 3 ]))
        {
            if ($column_match[ 3 ] !== '*')  $column_match[ 3 ] = '"' .  $column_match[ 3 ] . '"';
            return '"' . $this->prefix . $column_match[ 2 ] . '".' . $column_match[ 3 ];
        }
        return '"' . $string . '"';
    }
tds4u commented 7 years ago

Doesn't work. Problem is around line 386: $match:

Array(
  [0] => f.
  [column] => f.
  [1] => f.
)
preg_match('/(?<column>[a-zA-Z0-9_\.]+)(?:\s*\((?<alias>[a-zA-Z0-9_]+)\)|\s*\[(?<type>(String|Bool|Int|Number|Object|JSON))\])?/i', $value, $match);
Andrews54757 commented 7 years ago

@tds4u Then change the regex to

preg_match('/(?<column>[a-zA-Z0-9_\.\*]+)(?:\s*\((?<alias>[a-zA-Z0-9_]+)\)|\s*\[(?<type>(String|Bool|Int|Number|Object|JSON))\])?/i', $value, $match);
tds4u commented 7 years ago

That nearly works. Line 1015: Undefined index

case 'String':
    $stack[ $column_key ] = $data[ $column_key ];
    break;

Line 352:

protected function columnQuote($string)
{
    //preg_match('/(^#)?([a-zA-Z0-9_]*)\.([a-zA-Z0-9_]*)(\s*\[JSON\]$)?/', $string, $column_match);
    preg_match('/(^#)?([a-zA-Z0-9_]*)\.(\*|([a-zA-Z0-9_]*))(\s*\[JSON\]$)?/', $string, $column_match);
    if (isset($column_match[ 2 ], $column_match[ 3 ]))
    {
        if ($column_match[ 3 ] === '*') {
            return '"' . $this->prefix . $column_match[ 2 ] . '".' . $column_match[ 3 ];
        }
        return '"' . $this->prefix . $column_match[ 2 ] . '"."' . $column_match[ 3 ] . '"';
    }

    return '"' . $string . '"';
}

Line 386:

//preg_match('/(?<column>[a-zA-Z0-9_\.]+)(?:\s*\((?<alias>[a-zA-Z0-9_]+)\)|\s*\[(?<type>(String|Bool|Int|Number|Object|JSON))\])?/i', $value, $match);
preg_match('/(?<column>[a-zA-Z0-9_\.\*]+)(?:\s*\((?<alias>[a-zA-Z0-9_]+)\)|\s*\[(?<type>(String|Bool|Int|Number|Object|JSON))\])?/i', $value, $match);
catfan commented 7 years ago

No. You can not do that while you joining two or more tables. Because it will be conflicted if those table with same column name. Database engine did not know which column should be output. This will be error.

For Example:

account.user_id
account.user_name
account.description

book.user_id
book.name
book.description

If we join two table together with using user_id, and select account.* or book.*. At this case, the description from two table will be conflicted. You need to specify description is from account.description or book.description.

Or using alias:

[
    'account.user_id',
    'account.description',
    'book.description (book_desc)'
]

So, you may have to specify all column name while you joining tables, or find the way to get or cache the targeted column name at first.

Additionally, there too much article talking about that using * for getting all column will be slow down the query speed. You will get more unexpected data and using more memory.

salvationarinze commented 5 years ago

Use below function to dynamically construct table names in MYSQL

private function fetchColumnNames($table, $prepend=false) { $columns= $this->db->query('SHOW columns FROM '.$table)->fetchAll(PDO::FETCH_COLUMN); // manually constructing table_name.column_name if ($prepend) { $columns = array_map(function ($val) use ($table){ return $table.'.'.$val; }, $columns); } return $columns; }

You are welcome :)