Skip to content

Conversation

@foxycode
Copy link
Contributor

I don't understand why fetchField wasn't added to Selection earlier. I am missing it here every day. Hope this will be accepted.

I wanted to create some tests for Selection too, but I can't get existing tests running. Testers show me this: http://i.imgur.com/13EzLhg.png But I am sure that I have pdo installed and enabled.

@JanTvrdik
Copy link
Contributor

@foxycode You need to set php.ini http://tester.nette.org/#toc-vlastni-php-ini

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about select($column) before fetch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dg Makes sense, but shouldn't be then select added to fetchPairs too? And it will probably not work for int column key which is default for fetchField function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActiveRow supports indexed keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I see, it does not. So I can add select() as you proposed and make $column without default value. Or I can try to update ActiveRow to support indexed keys. What would you like reather?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActiveRow should not support indexed keys. $column default value can be NULL to allow $table->select('xx')->fetchSingle() too.

@foxycode
Copy link
Contributor Author

@JanTvrdik Thanks

@foxycode
Copy link
Contributor Author

@dg Let me know if this is sufficient. I'll try to write some test then.

@foxycode
Copy link
Contributor Author

Updated and with test.

@foxycode
Copy link
Contributor Author

@dg What should I do to push this further?

@dg
Copy link
Member

dg commented Aug 22, 2015

Thanks. Can you change commit message to Selection: added fetchField().

@foxycode
Copy link
Contributor Author

@dg Changed and rebased from current master

@dg
Copy link
Member

dg commented Aug 22, 2015

Thanks!

dg added a commit that referenced this pull request Aug 22, 2015
Added fetchField function to Selection
@dg dg merged commit 8b45f9f into nette:master Aug 22, 2015
@foxycode
Copy link
Contributor Author

@dg Glad to contribute. Hope you plan new release early :)

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

Successfully merging this pull request may close these issues.

3 participants