NULLS FIRST | NULLS LAST by SimpleForest · Pull Request #102 · rbock/sqlpp23

SimpleForest · 2026-02-22T12:21:34Z

Hello,

please let me know what you think of the general approach.

The static assert does not help. I can still compile ...nulls_first().nulls_first().
I implement it in core, not in postgresql speific module for the moment.

SimpleForest · 2026-02-22T12:24:18Z

I also have no clue, why here 3 commits are showing up. I am used to gitlab which handles git differently. I will clean up later.

SimpleForest · 2026-02-22T12:25:55Z

@MeanSquaredError Can you have a look here? I found finally some time to make a first attempt. You can see all in the third commit. The test is still messed up.

MeanSquaredError · 2026-02-22T15:30:06Z

Thanks for the PR, I will try to take a look at it shortly!

I also have no clue, why here 3 commits are showing up.

I think that you could try rebasing your branch against the latest main branch, e.g.

cd /path/to/your/copy/of/sqlpp
git remote add upstream https://github.com/rbock/sqlpp23.git
git pull --rebase upstream main
git push -f

This should synchronize the first two commits with the main branch and after the push they should disappear from this PR.

MeanSquaredError · 2026-02-22T17:39:16Z

OK, I rebased your branch on top of the latest main branch and then tried playing a bit with the nulls_first() and nulls_last() that you added.

I think it is OK to have the support for NULLS FIRST/NULLS LAST added to the core, since they are a part of the standard although their support is optional (according to Wikipedia they are described in SQL:2003 extension T611. Then the database connectors whose databases don't support it, could turn it off using some kind of type traits or a similar approach. I think right now PostgreSQL and SQLite3 support it and MySQL doesn't.

I wrote a basic build script + a very basic test program, which I will provide below in order to let everyone do the testing themselves.

build.sh
(you may need to adjust the paths in it)

#!/usr/bin/bash

g++ \
    -std=c++23 \
    -I/usr/local/projects/github/sqlpp23/include \
    -I/usr/local/projects/github/sqlpp23/tests/include/sqlpp23 \
    -Werror \
    -Wall \
    -Wextra \
    -Wpedantic \
mytest.cpp \
    -lpq

The actual test program, mytest.cpp

#include <tests/postgresql/make_test_connection.h>
#include <tests/postgresql/tables.h>

int main ()
{
	auto db = sqlpp::postgresql::make_test_connection ();
	auto tab = test::TabFoo {};
	db (
		select (tab.id)
		.from(tab)
//		.order_by (tab.id.asc ())
//		.order_by (tab.id.nulls_first ())
//		.order_by (tab.id.asc ().nulls_first ())
	);
	return 0;
}

The three commented-out lines correspond to three valid ORDER BY clauses:

ORDER BY id ASC
ORDER BY id NULLS FIRST
ORDER BY ASC NULLS FIRST

You can test each of them by removing the comment from the corresponding line in the source code. Although valid SQL, the first two compile but generate invalid SQL at runtime

This is the first (.order_by (tab.id.asc ())) at runtime:

connecting to the database server.
executing: 'SET TIME ZONE UTC;'
executing: 'SELECT tab_foo.id FROM tab_foo ORDER BY tab_foo.id ASCNULL'
terminate called after throwing an instance of 'sqlpp::postgresql::result_exception'
  what():  ERROR:  syntax error at or near "ASCNULL"
LINE 1: SELECT tab_foo.id FROM tab_foo ORDER BY tab_foo.id ASCNULL
                                                           ^

Aborted                    (core dumped) ./a.out

This is the second (.order_by (tab.id.nulls_first ())) at runtime:

connecting to the database server.
executing: 'SET TIME ZONE UTC;'
executing: 'SELECT tab_foo.id FROM tab_foo ORDER BY tab_foo.idNULL NULLS FIRST'
terminate called after throwing an instance of 'sqlpp::postgresql::result_exception'
  what():  ERROR:  column tab_foo.idnull does not exist
LINE 1: SELECT tab_foo.id FROM tab_foo ORDER BY tab_foo.idNULL NULLS...
                                                ^

Aborted                    (core dumped) ./a.out

The third one doesn't compile with g++ (GCC) 15.2.1 20260123 (Red Hat 15.2.1-7):

In file included from /usr/local/projects/github/sqlpp23/include/sqlpp23/core/query/dynamic.h:35,
                 from /usr/local/projects/github/sqlpp23/include/sqlpp23/core/basic/join_fwd.h:32,
                 from /usr/local/projects/github/sqlpp23/include/sqlpp23/core/basic/enable_join.h:30,
                 from /usr/local/projects/github/sqlpp23/include/sqlpp23/core/basic/table.h:32,
                 from /usr/local/projects/github/sqlpp23/include/sqlpp23/core/basic/schema_qualified_table.h:32,
                 from /usr/local/projects/github/sqlpp23/include/sqlpp23/sqlpp23.h:30,
                 from /usr/local/projects/github/sqlpp23/tests/include/sqlpp23/tests/postgresql/make_test_connection.h:37,
                 from mytest.cpp:1:
/usr/local/projects/github/sqlpp23/include/sqlpp23/core/operator/sort_order_expression.h: In instantiation of ‘constexpr sqlpp::sort_order_expression<L> sqlpp::sort_order_expression<L>::nulls_first() [with L = sqlpp::column_t<sqlpp::table_t<test::TabFoo_>, test::TabFoo_::Id>]’:
mytest.cpp:13:40:   required from here
   13 |                 .order_by (tab.id.asc ().nulls_first ())
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~^~
/usr/local/projects/github/sqlpp23/include/sqlpp23/core/operator/sort_order_expression.h:71:36: error: non-constant condition for static assertion
   71 |   static_assert(_rhs._rhs.has_value(), "nulls_first() can only be called when nulls_pos is not already set.");
      |                 ~~~~~~~~~~~~~~~~~~~^~
/usr/local/projects/github/sqlpp23/include/sqlpp23/core/operator/sort_order_expression.h:71:36:   in ‘constexpr’ expansion of ‘((sqlpp::sort_order_expression<sqlpp::column_t<sqlpp::table_t<test::TabFoo_>, test::TabFoo_::Id> >*)this)->sqlpp::sort_order_expression<sqlpp::column_t<sqlpp::table_t<test::TabFoo_>, test::TabFoo_::Id> >::_rhs.sqlpp::sort_order::_rhs.std::optional<sqlpp::nulls_pos>::has_value()’
/usr/include/c++/15/optional:1208:35:   in ‘constexpr’ expansion of ‘((const std::optional<sqlpp::nulls_pos>*)this)->std::optional<sqlpp::nulls_pos>::std::_Optional_base<sqlpp::nulls_pos, true, true>.std::_Optional_base<sqlpp::nulls_pos, true, true>::_M_is_engaged()’
/usr/local/projects/github/sqlpp23/include/sqlpp23/core/operator/sort_order_expression.h:71:17: error: ‘*(sqlpp::sort_order_expression<sqlpp::column_t<sqlpp::table_t<test::TabFoo_>, test::TabFoo_::Id> >*)this’ is not a constant expression
   71 |   static_assert(_rhs._rhs.has_value(), "nulls_first() can only be called when nulls_pos is not already set.");
      |                 ^~~~

I think that the main problem with the current implementation is that the type of the column order expression (sqlpp::sort_order_expression) does not encode any kind of information regarding the presence of direction (ASC or DESC) and nulls position (NULLS FIRST or NULLS LAST). Right now it just doesn't implement the asc() and desc() methods, which is fine if we support only direction (asc() or desc()), but if we want to support both direction and nulls position, then we need to somehow encode that in the type. So maybe sqlpp::sort_order_expression should have two additional boolean template parameters, e.g. HasDirection and HasNullsPosition, which we will use to decide if the methods asc(), desc(), nulls_first() and nulls_last() can be called on the corresponding sort expression.

The actual enabling/disabling of the methods can be done either through template specialization or through C++20 requires clauses attached to the corresponding methods. The approach with requires clauses seems easier to me, but I guess you need to try it to see whether it is simpler indeed.

Maybe @rbock could chime in and let us know if my proposed design seems OK to him, or maybe he can think of a better approach.

rbock · 2026-02-23T05:54:05Z

Thanks for the PR and the discussion so far.

I am a bit pressed for time, but quickly:

Disallowing nulls_first in the MySQL connector:
This is typically done in specializations of compatibility_check, see include/sqlpp23/mysql/constraints.h

asc/desc and nulls_first/last:
Currently, order_by expressions require asc or desc. I want to keep it this way. That means that .order_by(t.id.nulls_first()) would be invalid.

And then it is rather straight forward:

Calling .asc() or .desc() on an expression turns it into a sort_order_expression which offers .nulls_first() and .nulls_last(). Calling those would return a new type (with no .nulls_first/last).

Hope this makes sense?

Cheers,
Roland

MeanSquaredError · 2026-02-23T09:05:12Z

@rbock
Thanks for the clarification. Keeping the ordering direction (ASC/DESC) mandatory does simplify things indeed at the cost of being slightly more restrictive than the actual SQL standard.

like asc or desc can be more spefied via nulls first or last.

MeanSquaredError · 2026-03-05T04:29:37Z

include/sqlpp23/core/operator/sort_order_expression.h

+
 template <typename L>
 struct sort_order_expression {
-  constexpr sort_order_expression(L l, sort_type r)


It's better to keep the old parameter names (l and r) and not rename them unless it is really required. This will decrease the PR size and will simplify the PR review.

MeanSquaredError · 2026-03-05T04:32:02Z

Looks good so far!

rbock · 2026-03-05T05:55:02Z

Hey @SimpleForest , thanks for the next iteration!

I left a bunch of comments. Beyond that, please add

the constraint for MySQL.
tests for serialization and the constraint for MySQL as the bare minimum.
documentation.

rbock · 2026-03-05T05:23:42Z

include/sqlpp23/core/operator/sort_order_expression.h

  sort_order_expression& operator=(sort_order_expression&&) = default;
  ~sort_order_expression() = default;

+  constexpr auto nulls_first() -> sort_order_with_nulls_expression<L>;


Please stay aligned with the library style (functions are defined inline).

rbock · 2026-03-05T05:40:23Z

include/sqlpp23/core/operator/sort_order_expression.h

+
+ private:
+  friend reader_t;
+  const sort_order_expression<L> _lhs;


Please don't use const for the members.

These are private data members. Making them const does make the API any better.
But it could hurt if someone wanted to use copy or move assignment.

Also (general recommendation), it pays off to follow patterns of the existing code, e.g. for readability.

It might be perfectly fine to change the coding style, to introduce new patterns, etc, but that should happen in separate refactoring CLs.

rbock · 2026-03-05T05:43:04Z

include/sqlpp23/core/operator/enable_comparison.h

    return ::sqlpp::order(std::forward<Expr>(self), t);
  }

+  template <typename Expr>


This would go away, right? nulls_first and friends would only make sense in an order_expression.

rbock · 2026-03-05T05:45:29Z

include/sqlpp23/core/operator/comparison_functions.h

+  return {std::move(l), order};
+}
+
+enum class nulls_pos {


This should go to sort_order_expression.h now

rbock · 2026-03-05T05:46:08Z

include/sqlpp23/core/operator/comparison_functions.h

+  last,
+};
+
+template <typename L>


These should be dropped given the changes to sort_order_expression.

rbock · 2026-03-05T05:48:58Z

include/sqlpp23/core/operator/comparison_functions.h

 requires(values_are_comparable<L, L>::value)
 constexpr auto asc(L l) -> sort_order_expression<L> {
-  return {l, sort_type::asc};
+  return {std::move(l), sort_type::asc};


Once you undo the other changes in this file, these would stand out as unrelated refactoring.

These are good, but should go into a separate PR.

SimpleForest marked this pull request as draft February 22, 2026 12:24

We add a new struct to be able to express that a sort order

8e174bc

like asc or desc can be more spefied via nulls first or last.

SimpleForest force-pushed the main branch from 144aa50 to 8e174bc Compare March 4, 2026 19:10

MeanSquaredError reviewed Mar 5, 2026

View reviewed changes

MeanSquaredError mentioned this pull request Mar 5, 2026

Minor code cleanup questions #105

Open

rbock requested changes Mar 5, 2026

View reviewed changes

Conversation

SimpleForest commented Feb 22, 2026

Uh oh!

SimpleForest commented Feb 22, 2026

Uh oh!

SimpleForest commented Feb 22, 2026

Uh oh!

MeanSquaredError commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MeanSquaredError commented Feb 22, 2026

Uh oh!

rbock commented Feb 23, 2026

Uh oh!

MeanSquaredError commented Feb 23, 2026

Uh oh!

MeanSquaredError Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

MeanSquaredError commented Mar 5, 2026

Uh oh!

rbock commented Mar 5, 2026

Uh oh!

rbock Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rbock Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rbock Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rbock Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rbock Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rbock Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MeanSquaredError commented Feb 22, 2026 •

edited

Loading