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

Wrong metadata data types for PostgreSQL DB #12842

Closed
tztztztz opened this Issue May 10, 2017 · 26 comments

Comments

Projects
None yet
3 participants
@tztztztz
Contributor

tztztztz commented May 10, 2017

Method \Phalcon\Mvc\Model\MetaData::getDataTypes is returning wrong datatype for PostgreSQL database and column types double precision and real.

It is returning 2 which wrongly corresponding to \Phalcon\Db\Column ::TYPE_VARCHAR instead of respectively: \Phalcon\Db\Column::TYPE_DOUBLE for double precision and \Phalcon\Db\Column::TYPE_FLOAT for real.

Example dump of print_r of array returned by getDataTypes($model), where $model is subclass of \Phalcon\Mvc\Model:

$dataTypes = $entity->getModelsMetaData()->getDataTypes($entity);
echo '<pre>'; print_r($dataTypes); exit;
Array
(
    [id] => 0
    [name] => 2
    [is_self_delivery] => 8
    [is_active_for_client] => 8
    [is_active_for_admin] => 8
    [default_delivery_price_netto] => 2
    [is_default] => 8
    [can_delivery_to_client] => 8
    [can_delivery_returns] => 8
    [php_manager_class] => 2
    [default_delivery_price_vat_pct] => 2
    [default_delivery_price_vat_val] => 2
    [default_delivery_price_brutto] => 2
)

Table structure dump from Postgresql version 9.5:

CREATE TABLE public.k3_delivery_method
(
  id integer NOT NULL DEFAULT nextval('k3_delivery_method_id_seq'::regclass),
  name character varying(128),
  is_self_delivery boolean NOT NULL DEFAULT false,
  is_active_for_client boolean NOT NULL DEFAULT false,
  is_active_for_admin boolean NOT NULL DEFAULT false,
  default_delivery_price_netto double precision NOT NULL,
  is_default boolean NOT NULL DEFAULT false,
  can_delivery_to_client boolean NOT NULL DEFAULT true,
  can_delivery_returns boolean NOT NULL DEFAULT false,
  php_manager_class character varying(255),
  default_delivery_price_vat_pct real NOT NULL DEFAULT 0,
  default_delivery_price_vat_val double precision NOT NULL DEFAULT 0,
  default_delivery_price_brutto double precision NOT NULL DEFAULT 0,
  CONSTRAINT k3_delivery_method_pkey PRIMARY KEY (id)
)
WITH (
  OIDS=FALSE
);
ALTER TABLE public.k3_delivery_method
  OWNER TO postgres;

System details:

PHALCON:
Web framework delivered as a C-extension for PHP
phalcon => enabled
Author => Phalcon Team and contributors
Version => 3.1.2
Build Date => Apr 5 2017 15:48:35
Powered by Zephir => Version 0.9.7-1fae5e50ac

Directive => Local Value => Master Value
phalcon.db.escape_identifiers => On => On
phalcon.db.force_casting => Off => Off
phalcon.orm.events => On => On
phalcon.orm.virtual_foreign_keys => On => On
phalcon.orm.column_renaming => On => On
phalcon.orm.not_null_validations => On => On
phalcon.orm.exception_on_failed_save => Off => Off
phalcon.orm.enable_literals => On => On
phalcon.orm.late_state_binding => Off => Off
phalcon.orm.enable_implicit_joins => On => On
phalcon.orm.cast_on_hydrate => Off => Off
phalcon.orm.ignore_unknown_columns => Off => Off

PHP:
PHP 5.6.21 (cli) (built: Apr 27 2016 20:13:58)
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies

System:
Windows 10

Webserver:
Nginx

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Is anyone up to fix this problem? It should be easy to do so....

Contributor

tztztztz commented Jul 13, 2017

Is anyone up to fix this problem? It should be easy to do so....

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Jul 13, 2017

Member

You can always create PR if you want.

Member

Jurigag commented Jul 13, 2017

You can always create PR if you want.

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Whats PR?

Contributor

tztztztz commented Jul 13, 2017

Whats PR?

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Jul 13, 2017

Member

Pull Request

Member

Jurigag commented Jul 13, 2017

Pull Request

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

I dont know how to do pull request and I can't use Zephyr on windows.

Can you fix it? This is like 5 min job for someone who have Zephyr ready. You dont even need Postgresql for that.

here's list of datatypes:
https://www.postgresql.org/docs/current/static/datatype.html#DATATYPE-TABLE

Just do mapping for every type like:

bigint => ....
double precision => ...
real => ...

etc.

Contributor

tztztztz commented Jul 13, 2017

I dont know how to do pull request and I can't use Zephyr on windows.

Can you fix it? This is like 5 min job for someone who have Zephyr ready. You dont even need Postgresql for that.

here's list of datatypes:
https://www.postgresql.org/docs/current/static/datatype.html#DATATYPE-TABLE

Just do mapping for every type like:

bigint => ....
double precision => ...
real => ...

etc.

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Jul 13, 2017

Member

You don't need zephir for it. You never used git/github? Oh okay.

Member

Jurigag commented Jul 13, 2017

You don't need zephir for it. You never used git/github? Oh okay.

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

No, I have account at github because it's required by Phalcon Phorum, that's the only reason.

Sorry, but I doesn't understand why I don't need Zephyr - source code of Phalcon is written in Zephyr, so in order to fix it, I need to use Zephyr, right, or not?

Contributor

tztztztz commented Jul 13, 2017

No, I have account at github because it's required by Phalcon Phorum, that's the only reason.

Sorry, but I doesn't understand why I don't need Zephyr - source code of Phalcon is written in Zephyr, so in order to fix it, I need to use Zephyr, right, or not?

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Jul 13, 2017

Member

Yes but you can use any editor to edit zephir code, you don't need to install it to fix anything.

Member

Jurigag commented Jul 13, 2017

Yes but you can use any editor to edit zephir code, you don't need to install it to fix anything.

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Ok, but how can i test it? You want me to write code without compiling and testing it?

Contributor

tztztztz commented Jul 13, 2017

Ok, but how can i test it? You want me to write code without compiling and testing it?

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Jul 13, 2017

Member

By making PR, it will compile and test on travis. I mean yea you can compile and test yourself, but it's not required, travis can do it. I many times make some PR with fix or new feature without compiling and testing it myself, it would cost me too much time.

Member

Jurigag commented Jul 13, 2017

By making PR, it will compile and test on travis. I mean yea you can compile and test yourself, but it's not required, travis can do it. I many times make some PR with fix or new feature without compiling and testing it myself, it would cost me too much time.

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

I can't do it, i don't know github, it would take probably several hours for me before i woud find out how to create pull request. Why you can't fix it?

Contributor

tztztztz commented Jul 13, 2017

I can't do it, i don't know github, it would take probably several hours for me before i woud find out how to create pull request. Why you can't fix it?

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Jul 13, 2017

Member

I can fix it, just don't have time right now for it. What you mean how? Just by making PR.

  1. Fork repo
  2. Edit code in repository
  3. https://help.github.com/articles/creating-a-pull-request/

Pretty simple and easy really. I will try to fix it, but not sure when, maybe will try to look into it on weekend.

Member

Jurigag commented Jul 13, 2017

I can fix it, just don't have time right now for it. What you mean how? Just by making PR.

  1. Fork repo
  2. Edit code in repository
  3. https://help.github.com/articles/creating-a-pull-request/

Pretty simple and easy really. I will try to fix it, but not sure when, maybe will try to look into it on weekend.

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Sorry but im unable to do it. I tryied but failed. Ok then, bug stays.

Contributor

tztztztz commented Jul 13, 2017

Sorry but im unable to do it. I tryied but failed. Ok then, bug stays.

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Is this file responsible for mapping or im wrong:

https://github.com/tztztztz/cphalcon/blob/master/phalcon/mvc/model/metadata/strategy/annotations.zep

Is it for annotations only? Where I can find mapping?

Contributor

tztztztz commented Jul 13, 2017

Is this file responsible for mapping or im wrong:

https://github.com/tztztztz/cphalcon/blob/master/phalcon/mvc/model/metadata/strategy/annotations.zep

Is it for annotations only? Where I can find mapping?

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Come on at least tell me what file i must change.

Contributor

tztztztz commented Jul 13, 2017

Come on at least tell me what file i must change.

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Is there any way to search entire forked code?

Contributor

tztztztz commented Jul 13, 2017

Is there any way to search entire forked code?

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag
Member

Jurigag commented Jul 13, 2017

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Thanks

Contributor

tztztztz commented Jul 13, 2017

Thanks

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Ok let's see what happens now :|

Contributor

tztztztz commented Jul 13, 2017

Ok let's see what happens now :|

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Jul 13, 2017

Member

You need to use 3.2.x branch and also update changelog.md

Member

Jurigag commented Jul 13, 2017

You need to use 3.2.x branch and also update changelog.md

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

I dont know how to do it

Contributor

tztztztz commented Jul 13, 2017

I dont know how to do it

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

but i changed the code, can you pull request for me copying code?

Contributor

tztztztz commented Jul 13, 2017

but i changed the code, can you pull request for me copying code?

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Just copy my work and pull request ok?

Contributor

tztztztz commented Jul 13, 2017

Just copy my work and pull request ok?

@tztztztz

This comment has been minimized.

Show comment
Hide comment
@tztztztz

tztztztz Jul 13, 2017

Contributor

Is it ok now...? I lost so much time learning how to do it, just to make small change :/

Contributor

tztztztz commented Jul 13, 2017

Is it ok now...? I lost so much time learning how to do it, just to make small change :/

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Jul 17, 2017

Member

Fixed in the 3.2.x branch. Feel free to open a new issue if the problem appears again. Thank you for contributing.

Member

sergeyklay commented Jul 17, 2017

Fixed in the 3.2.x branch. Feel free to open a new issue if the problem appears again. Thank you for contributing.

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Apr 16, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

stale bot commented Apr 16, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Apr 16, 2018

@sergeyklay sergeyklay closed this Apr 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment