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

[YSQL] Introduce the PGConn::FetchAll helper method #19075

Closed
1 task done
d-uspenskiy opened this issue Sep 11, 2023 · 0 comments
Closed
1 task done

[YSQL] Introduce the PGConn::FetchAll helper method #19075

d-uspenskiy opened this issue Sep 11, 2023 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage
Projects

Comments

@d-uspenskiy
Copy link
Contributor

d-uspenskiy commented Sep 11, 2023

Jira Link: DB-7889

Description

There are lots of unit tests which explicitly checks number of rows/columns in fetch result.

  PGResultPtr res = ASSERT_RESULT(conn_->Fetch(query));
  ASSERT_EQ(PQntuples(res.get()), 3);
  ASSERT_EQ(PQnfields(res.get()), 3);
  std::array<int, 3> values = {
    ASSERT_RESULT(GetInt32(res.get(), 0, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 1, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 2, 1)),
  };
  ASSERT_EQ(values[0], 0);
  ASSERT_EQ(values[1], 100);
  ASSERT_EQ(values[2], -5);

The PGConn class has the FetchMatrix method which can reduce some boiler plate code.

  PGResultPtr res = ASSERT_RESULT(conn_->FetchMatrix(query, 3, 3));
  std::array<int, 3> values = {
    ASSERT_RESULT(GetInt32(res.get(), 0, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 1, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 2, 1)),
  };
  ASSERT_EQ(values[0], 0);
  ASSERT_EQ(values[1], 100);
  ASSERT_EQ(values[2], -5);

But it will be good to have additional helper method which reads all the rows as a vector of tuples. This helper method will help to simplify code even more

const auto values = ASSERT_RESULT((conn_->FetchAll<char, int>(query)));
decltype(values) expected_values = {{'a', 0}, {'b', 100}, {'y', -5}};
ASSERT_EQ(values, expected_values);

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@d-uspenskiy d-uspenskiy added kind/enhancement This is an enhancement of an existing feature area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Sep 11, 2023
@d-uspenskiy d-uspenskiy added this to Backlog in YSQL via automation Sep 11, 2023
@d-uspenskiy d-uspenskiy self-assigned this Sep 11, 2023
@yugabyte-ci yugabyte-ci added the priority/medium Medium priority issue label Sep 11, 2023
@d-uspenskiy d-uspenskiy changed the title [YSQL] Introduce PGConn::FetchAll helper method [YSQL] Introduce the PGConn::FetchAll helper method Sep 11, 2023
d-uspenskiy added a commit that referenced this issue Sep 12, 2023
Summary:
There are lots of unit tests which explicitly checks number of rows/columns in fetch result.

```
  PGResultPtr res = ASSERT_RESULT(conn_->Fetch(query));
  ASSERT_EQ(PQntuples(res.get()), 3);
  ASSERT_EQ(PQnfields(res.get()), 3);
  std::array<int, 3> values = {
    ASSERT_RESULT(GetInt32(res.get(), 0, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 1, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 2, 1)),
  };
  ASSERT_EQ(values[0], 0);
  ASSERT_EQ(values[1], 100);
  ASSERT_EQ(values[2], -5);
```

The `PGConn` class has the `FetchMatrix` method which can reduce some boiler plate code.

```
  PGResultPtr res = ASSERT_RESULT(conn_->FetchMatrix(query, 3, 3));
  std::array<int, 3> values = {
    ASSERT_RESULT(GetInt32(res.get(), 0, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 1, 1)),
    ASSERT_RESULT(GetInt32(res.get(), 2, 1)),
  };
  ASSERT_EQ(values[0], 0);
  ASSERT_EQ(values[1], 100);
  ASSERT_EQ(values[2], -5);
```

This diff introduces new helper method `PGConn::FetchAll` which helps to simplify code even more

```
const auto values = ASSERT_RESULT((conn_->FetchAll<char, int>(query)));
decltype(values) expected_values = {{'a', 0}, {'b', 100}, {'y', -5}};
ASSERT_EQ(values, expected_values);
```

The method reads all the rows from result with respect to column's type as vector of tuples

Also this diff adds support for `char` type in the `GetValue<...>` function.

**Note:**
This diff simplifies one unit test just for usage example. Other unit tests will be simplified by further diffs step by step.
Jira: DB-7889

Test Plan: Jenkins

Reviewers: sergei, jason

Reviewed By: sergei, jason

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D28454
YSQL automation moved this from Backlog to Done Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage
Projects
YSQL
  
Done
Development

No branches or pull requests

2 participants