Bug: cannot use table alias in Model

[wiedhodho]

I use $model->table('kategori k1')->select('k1.*')

But error: Unknown table dbname.k1

CodeIgniter 4 version v4.1.1

[avegacms]

It's not bug! You must write your alias as $model->table('kategori AS k1')->select('k1.*')

[wiedhodho]

I have try with $model->table('kategori as k1')

but the error is still same.

database: mariadb v10.5.8

[MGatner]

Without digging through the code, I'm not sure we support this? You might need to use Builder's "table alias" methods. I really don't have experience with this issue though.

[kenjis]

Not $model->table('kategori k1')->select('k1.*') but $db->table('kategori k1')->select('k1.*')? It seems CodeIgniter\Model does not have the method table().

[MGatner]

@kenjis Model has mixins to both Builder and Connection so table() is acceptable.

[kenjis]

Oh, BaseModel has mixin to Connection. The model is a really complex class!

[MGatner]

It's really a mess. There's not a need for 90% of this magic and it makes docs and static analysis pretty confusing. I will lobby to simplify all this for the next major version but for now I think the best we can do is try to get SA working as-is.

[WinterSilence]

@MGatner we can use PhpDoc to fix it, see @method tag

[MGatner]

@WinterSilence I still have not looked into the templating you linked on the other issue but I fear that a simple @method tag won't be enough given the underlying complexity.

[paulbalandan]

I think this is related to #4527

[kenjis]

@paulbalandan Probably no.

$model->table('kategori k1') does not set $model->builder. After that, when $model->builder() is called, another builder is created and it is set $model->builder.

[kenjis]

I'm not sure $model->table('kategori k1') should be supported.

The following code would work:

$model->bulder(); // set builder in model
$model->from('kategori k1', true)->select('k1.*');

[WinterSilence]

@kenjis wait, somebody will write about BC - 💯 (:

[kenjis]

Wait for what? I just showed workaround code for this issue.

To be honest, I don't know what CodeIgniter\Model is trying to achieve.

Currently, the magic method in the Model can execute all BaseConnection and BaseBuilder methods, so the BaseConnection::table() method can also be executed.

But do we need to support the usage like $model->table('tablename')?

[WinterSilence]

@kenjis $model->from('kategori k1', true)->select('k1.*'); auto create builder.

But do we need to support the usage like $model->table('tablename')?

remove = BC

[MGatner]

We can't do anything about the database magic pass-through right now but definitely let's remove it in the future. It makes such a mess, when a simple accessor to $db would allow the same thing, just cleaner. The Builder magic is a bit more complicated, and we will probably need to find some way to keep that. Hopefully removing magic methods from Builder will help with the clarity and static analysis.

[WinterSilence]

need change all without changing anything

i think need rewrite classes in new namespace and mark current classes as @deprecated

namespace CodeIgniter\Models;

interface ModelInterface extends \IteratorAggregate
{
    public function getValidation(): Validation;
    public function getEventManager(): EventManager;
    public function getFields(): FieldCollection;
}

interface StoredModelInterface extends ModelInterface 
{
    public function getStorage(): StorageProviderInterface;
    public function save(StorageProviderInterface $storage = null): bool;
}

interface FieldInterface
{
    public function getId(): string;
    public function getName(): string;
    public function getModel(): ModelInterface;
    public function  setValidationRules(array $rules);
    public function setEventListeners(string $event, callable ...$listeners);
    public function getValue();
    public function setValue($value);
    public function bindValue(&$value);
}

class FieldCollection implements \IteratorAggregate
{
    public function __construct(FieldInterface ...$fields)
    {
          foreach ($fields as $field) {
               $this->fields[$field->getId()] => $field;
          }
    }
    public function toAll(): array {
         return $this->fields;
    }
    public function get(string $id): FieldInterface {}
    public function getValidationRules(): array {}
    public function getEventListeners(): array {}
    public function getValues(): array {}
    public function setValues(array $values): {}
    public function has(string $field): bool {}
    public function getIterator(): \Iterator
    {
        yield from $this->fields;
    }
}

abstract class Model implements ModelInterface
{
        protected $fields;

        protected $validation;

        protected $eventManager;

       public function getFields(): FieldCollection
       {
               if ($this->fields === null) {
                       $this->fields = new FieldCollection($this);
               } 
               return $this->fields;
       }

        public function __construct()
        {
             $this->getEventManager()->dispatch(new ModelInitEvent($this));
        }

        abstract protected function getEventListeners(): array;

       public function getValidation(): Validation
       {
               if ($this->validation === null) {
                       $this->validation= new Validation($this);
                       $this->validation->addRules($this->getFields()->getValidationRules());
               }
               return $this->validation;
       }

        public function getEventManager(): EventManager
        {
            if ($this->eventManager === null) {
                  $this->eventManager = new EventManager();
                  $this->eventManager->addListeners($this->getEventListeners());
                  $this->eventManager->addListeners($this->getFields()->getEventListeners());
            }
            return $this->eventManager;
        }

      public function __set(string $name, $value)
      {
            throw new \RuntimeException('Undefined property: ' . $name);
      }

      public function getIterator(): \Iterator
      {
          yield from $this->getFields();
      }
}

abstract class StoredModel extends Model implements StoredModelInterface
{
}

namespace CodeIgniter\Database;
abstract class BaseModel extends StoredModel
{
    abstract public function getTable(): Table;
    abstract protected function getQueryBuilder(): QueryBuilder;
    abstract public function getRelations(): RelationCollection;
}

[kenjis]

@WinterSilence

$model->from('kategori k1', true)->select('k1.*'); auto create builder.

Sorry, I don't understand what you mean.

[WinterSilence]

@kenjis https://github.com/codeigniter4/CodeIgniter4/blob/5088795c394929346b054e1a0204fc946a01fe77/system/Model.php#L784

[kenjis]

$model->bulder(); // set builder in model
$model->from('kategori k1', true)->select('k1.*');

The first line makes a builder and set it to $this->builder in the Model. So the second line does not create a new builder.

[WinterSilence]

@kenjis when you call method from() you set builder in __call():

if ($result === null && method_exists($builder = $this->builder(), $name))
{
      $result = $builder->{$name}(...$params);
}

you must call methods by chain : $model->bulder()->select('k1.*')->from('kategori k1', true)

[kenjis]

@wiedhodho For what do you want to write code like $model->table('kategori k1')->select('k1.*')?

Why not like this?

namespace App\Models;
class KategoriModel extends \CodeIgniter\Model
{
    protected $table = 'kategori';
}

$model->select('*')
or 
$model>findAll()

[kenjis]

I don't think we should support $model->table(). The CodeIgniter Model is not Query Builder.

[kenjis]

No feedback. Closing.